diff --git a/packages/ember-glimmer/lib/environment.js b/packages/ember-glimmer/lib/environment.js index 7a864b106b4..f7a74543726 100644 --- a/packages/ember-glimmer/lib/environment.js +++ b/packages/ember-glimmer/lib/environment.js @@ -25,6 +25,7 @@ import { SimpleHelperReference, ClassBasedHelperReference } from './utils/references'; +import DebugStack from './utils/debug-stack'; import { inlineIf, @@ -133,6 +134,8 @@ export default class Environment extends GlimmerEnvironment { '-html-safe': htmlSafeHelper, '-get-dynamic-var': getDynamicVar }; + + runInDebug(() => this.debugStack = new DebugStack()); } // Hello future traveler, welcome to the world of syntax refinement. diff --git a/packages/ember-glimmer/lib/syntax/curly-component.js b/packages/ember-glimmer/lib/syntax/curly-component.js index 4ab0d8fe56e..5b64026ae9a 100644 --- a/packages/ember-glimmer/lib/syntax/curly-component.js +++ b/packages/ember-glimmer/lib/syntax/curly-component.js @@ -186,6 +186,8 @@ class CurlyComponentManager { } create(environment, definition, args, dynamicScope, callerSelfRef, hasBlock) { + runInDebug(() => this._pushToDebugStack(`component:${definition.name}`, environment)); + let parentView = dynamicScope.view; let factory = definition.ComponentClass; @@ -305,6 +307,8 @@ class CurlyComponentManager { didRenderLayout(bucket, bounds) { bucket.component[BOUNDS] = bounds; bucket.finalize(); + + runInDebug(() => this.debugStack.pop()); } getTag({ component }) { @@ -322,6 +326,8 @@ class CurlyComponentManager { update(bucket, _, dynamicScope) { let { component, args, argsRevision, environment } = bucket; + runInDebug(() => this._pushToDebugStack(component._debugContainerKey, environment)); + bucket.finalizer = _instrumentStart('render.component', rerenderInstrumentDetails, component); if (!args.tag.validate(argsRevision)) { @@ -362,12 +368,21 @@ class CurlyComponentManager { } } +runInDebug(() => { + CurlyComponentManager.prototype._pushToDebugStack = function(name, environment) { + environment.debugStack.push(name); + this.debugStack = environment.debugStack; + }; +}); + const MANAGER = new CurlyComponentManager(); class TopComponentManager extends CurlyComponentManager { create(environment, definition, args, dynamicScope, currentScope, hasBlock) { let component = definition.ComponentClass.create(); + runInDebug(() => this._pushToDebugStack(component._debugContainerKey, environment)); + let finalizer = _instrumentStart('render.component', initialRenderInstrumentDetails, component); dynamicScope.view = component; diff --git a/packages/ember-glimmer/lib/syntax/mount.js b/packages/ember-glimmer/lib/syntax/mount.js index 34dc1d70727..6a29dc6f85f 100644 --- a/packages/ember-glimmer/lib/syntax/mount.js +++ b/packages/ember-glimmer/lib/syntax/mount.js @@ -8,7 +8,7 @@ import { ComponentDefinition } from 'glimmer-runtime'; import { UNDEFINED_REFERENCE } from 'glimmer-reference'; -import { assert } from 'ember-metal'; +import { assert, runInDebug } from 'ember-metal'; import { RootReference } from '../utils/references'; import { generateControllerFactory } from 'ember-routing'; import { OutletLayoutCompiler } from './outlet'; @@ -74,6 +74,8 @@ class MountManager { } create(environment, { name, env }, args, dynamicScope) { + runInDebug(() => this._pushToDebugStack(name, env)); + dynamicScope.outletState = UNDEFINED_REFERENCE; let engine = env.owner.buildChildEngineInstance(name); @@ -103,13 +105,24 @@ class MountManager { } didCreateElement() {} - didRenderLayout() {} + + didRenderLayout() { + runInDebug(() => this.debugStack.pop()); + } + didCreate(state) {} update(state, args, dynamicScope) {} didUpdateLayout() {} didUpdate(state) {} } +runInDebug(() => { + MountManager.prototype._pushToDebugStack = function(name, environment) { + this.debugStack = environment.debugStack; + this.debugStack.pushEngine(name); + }; +}); + const MOUNT_MANAGER = new MountManager(); class MountDefinition extends ComponentDefinition { diff --git a/packages/ember-glimmer/lib/syntax/outlet.js b/packages/ember-glimmer/lib/syntax/outlet.js index f20455ca7de..81aaa07d6cf 100644 --- a/packages/ember-glimmer/lib/syntax/outlet.js +++ b/packages/ember-glimmer/lib/syntax/outlet.js @@ -8,7 +8,7 @@ import { StatementSyntax, ComponentDefinition } from 'glimmer-runtime'; -import { _instrumentStart } from 'ember-metal'; +import { runInDebug, _instrumentStart } from 'ember-metal'; import { RootReference } from '../utils/references'; import { UpdatableTag, @@ -180,6 +180,8 @@ class OutletComponentManager { } create(environment, definition, args, dynamicScope) { + runInDebug(() => this._pushToDebugStack(definition, environment)); + let outletStateReference = dynamicScope.outletState = dynamicScope.outletState.get('outlets').get(definition.outletName); let outletState = outletStateReference.value(); return new StateBucket(outletState); @@ -203,6 +205,8 @@ class OutletComponentManager { didRenderLayout(bucket) { bucket.finalize(); + + runInDebug(() => this.debugStack.pop()); } didCreateElement() {} @@ -212,10 +216,19 @@ class OutletComponentManager { didUpdate(state) {} } +runInDebug(() => { + OutletComponentManager.prototype._pushToDebugStack = function(definition, environment) { + environment.debugStack.push(definition.template.meta.moduleName); + this.debugStack = environment.debugStack; + }; +}); + const MANAGER = new OutletComponentManager(); class TopLevelOutletComponentManager extends OutletComponentManager { create(environment, definition, args, dynamicScope) { + runInDebug(() => this._pushToDebugStack(definition, environment)); + return new StateBucket(dynamicScope.outletState.value()); } diff --git a/packages/ember-glimmer/lib/syntax/render.js b/packages/ember-glimmer/lib/syntax/render.js index 995248748d0..0f5eb3bf5f2 100644 --- a/packages/ember-glimmer/lib/syntax/render.js +++ b/packages/ember-glimmer/lib/syntax/render.js @@ -8,7 +8,7 @@ import { ComponentDefinition } from 'glimmer-runtime'; import { ConstReference, isConst } from 'glimmer-reference'; -import { assert } from 'ember-metal'; +import { assert, runInDebug } from 'ember-metal'; import { RootReference } from '../utils/references'; import { generateController, generateControllerFactory } from 'ember-routing'; import { OutletLayoutCompiler } from './outlet'; @@ -168,17 +168,30 @@ class AbstractRenderManager { didUpdate() {} } +runInDebug(() => { + AbstractRenderManager.prototype._pushToDebugStack = function(controllerName, environment) { + this.debugStack = environment.debugStack; + this.debugStack.push(`controller:${controllerName} (with render helper)`); + }; +}); + class SingletonRenderManager extends AbstractRenderManager { create(environment, definition, args, dynamicScope) { let { name, env } = definition; let controller = env.owner.lookup(`controller:${name}`) || generateController(env.owner, name); + runInDebug(() => this._pushToDebugStack(name, environment)); + if (dynamicScope.rootOutletState) { dynamicScope.outletState = dynamicScope.rootOutletState.getOrphan(name); } return { controller }; } + + didRenderLayout() { + runInDebug(() => this.debugStack.pop()); + } } const SINGLETON_RENDER_MANAGER = new SingletonRenderManager(); @@ -192,6 +205,8 @@ class NonSingletonRenderManager extends AbstractRenderManager { let factory = controllerFactory || generateControllerFactory(env.owner, name); let controller = factory.create({ model: modelRef.value() }); + runInDebug(() => this._pushToDebugStack(name, environment)); + if (dynamicScope.rootOutletState) { dynamicScope.outletState = dynamicScope.rootOutletState.getOrphan(name); } @@ -203,6 +218,10 @@ class NonSingletonRenderManager extends AbstractRenderManager { controller.set('model', args.positional.at(0).value()); } + didRenderLayout() { + runInDebug(() => this.debugStack.pop()); + } + getDestructor({ controller }) { return controller; } diff --git a/packages/ember-glimmer/lib/utils/debug-stack.js b/packages/ember-glimmer/lib/utils/debug-stack.js new file mode 100644 index 00000000000..66c08e42347 --- /dev/null +++ b/packages/ember-glimmer/lib/utils/debug-stack.js @@ -0,0 +1,62 @@ +import { runInDebug } from 'ember-metal'; + +let DebugStack; + +runInDebug(function() { + class Element { + constructor(name) { + this.name = name; + } + } + + class TemplateElement extends Element { } + class EngineElement extends Element { } + + DebugStack = class DebugStack { + constructor() { + this._stack = []; + } + + push(name) { + this._stack.push(new TemplateElement(name)); + } + + pop() { + this._stack.pop(); + } + + currentTemplate() { + return this._getCurrentByType(TemplateElement); + } + + pushEngine(name) { + this._stack.push(new EngineElement(name)); + } + + currentEngine() { + return this._getCurrentByType(EngineElement); + } + + peek() { + let template = this.currentTemplate(); + let engine = this.currentEngine(); + + if (engine) { + return `"${template}" (in the "${engine}" engine)`; + } else { + return `"${template}"`; + } + } + + _getCurrentByType(type) { + for (let i = this._stack.length; i >= 0; i--) { + let element = this._stack[i]; + if (element instanceof type) { + return element.name; + } + } + } + }; +}); + +export default DebugStack; diff --git a/packages/ember-glimmer/tests/integration/application/rendering-test.js b/packages/ember-glimmer/tests/integration/application/rendering-test.js index c063cc279e6..6ca7e68303f 100644 --- a/packages/ember-glimmer/tests/integration/application/rendering-test.js +++ b/packages/ember-glimmer/tests/integration/application/rendering-test.js @@ -2,6 +2,8 @@ import { Controller } from 'ember-runtime'; import { moduleFor, ApplicationTest } from '../../utils/test-case'; import { strip } from '../../utils/abstract-test-case'; import { Route } from 'ember-routing'; +import { isFeatureEnabled } from 'ember-metal'; +import { Component } from 'ember-glimmer'; moduleFor('Application test: rendering', class extends ApplicationTest { @@ -372,4 +374,41 @@ moduleFor('Application test: rendering', class extends ApplicationTest { }); }); } + + ['@test it emits a useful backtracking re-render assertion message'](assert) { + this.router.map(function() { + this.route('routeWithError'); + }); + + this.registerRoute('routeWithError', Route.extend({ + model() { + return { name: 'Alex' }; + } + })); + + this.registerTemplate('routeWithError', 'Hi {{model.name}} {{x-foo person=model}}'); + + this.registerComponent('x-foo', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + this.set('person.name', 'Ben'); + } + }), + template: 'Hi {{person.name}} from component' + }); + + let expectedBacktrackingMessage = /modified "model\.name" twice on \[object Object\] in a single render\. It was rendered in "routeWithError" and modified in "component:x-foo"/; + + if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + expectDeprecation(expectedBacktrackingMessage); + return this.visit('/routeWithError'); + } else { + return this.visit('/').then(() => { + expectAssertion(() => { + this.visit('/routeWithError'); + }, expectedBacktrackingMessage); + }); + } + } }); diff --git a/packages/ember-glimmer/tests/integration/components/curly-components-test.js b/packages/ember-glimmer/tests/integration/components/curly-components-test.js index 687d94fcf21..616a1a01b14 100644 --- a/packages/ember-glimmer/tests/integration/components/curly-components-test.js +++ b/packages/ember-glimmer/tests/integration/components/curly-components-test.js @@ -2075,10 +2075,6 @@ moduleFor('Components test: curly components', class extends RenderingTest { } ['@test when a property is changed during children\'s rendering'](assert) { - if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { - expectDeprecation(/modified value twice on <\(.+> in a single render/); - } - let outer, middle; this.registerComponent('x-outer', { @@ -2123,14 +2119,17 @@ moduleFor('Components test: curly components', class extends RenderingTest { assert.equal(this.$('#inner-value').text(), '1', 'initial render of inner'); assert.equal(this.$('#middle-value').text(), '', 'initial render of middle (observers do not run during init)'); - if (!isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + let expectedBacktrackingMessage = /modified "value" twice on <\(.+> in a single render\. It was rendered in "component:x-middle" and modified in "component:x-inner"/; + + if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + expectDeprecation(expectedBacktrackingMessage); + this.runTask(() => outer.set('value', 2)); + } else { expectAssertion(() => { this.runTask(() => outer.set('value', 2)); - }, /modified value twice on <\(.+> in a single render/); + }, expectedBacktrackingMessage); return; - } else { - this.runTask(() => outer.set('value', 2)); } assert.equal(this.$('#inner-value').text(), '2', 'second render of inner'); @@ -2148,10 +2147,6 @@ moduleFor('Components test: curly components', class extends RenderingTest { } ['@test when a shared dependency is changed during children\'s rendering'](assert) { - if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { - expectDeprecation(/modified wrapper.content twice on in a single render/); - } - let outer; this.registerComponent('x-outer', { @@ -2176,14 +2171,17 @@ moduleFor('Components test: curly components', class extends RenderingTest { template: '
{{wrapper.content}}
' }); - if (!isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + let expectedBacktrackingMessage = /modified "wrapper\.content" twice on in a single render\. It was rendered in "component:x-outer" and modified in "component:x-inner"/; + + if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + expectDeprecation(expectedBacktrackingMessage); + this.render('{{x-outer}}'); + } else { expectAssertion(() => { this.render('{{x-outer}}'); - }, /modified wrapper.content twice on in a single render/); + }, expectedBacktrackingMessage); return; - } else { - this.render('{{x-outer}}'); } assert.equal(this.$('#inner-value').text(), '1', 'initial render of inner'); diff --git a/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js b/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js index 55e68b89ece..6fb437d7163 100644 --- a/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js +++ b/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js @@ -2,6 +2,7 @@ import { set, computed } from 'ember-metal'; import { Component } from '../../utils/helpers'; import { strip } from '../../utils/abstract-test-case'; import { moduleFor, RenderingTest } from '../../utils/test-case'; +import { isFeatureEnabled } from 'ember-metal'; moduleFor('Components test: dynamic components', class extends RenderingTest { @@ -706,4 +707,37 @@ moduleFor('Components test: dynamic components', class extends RenderingTest { this.assertText('Foo4'); } + + ['@test component helper emits useful backtracking re-render assertion message'](assert) { + this.registerComponent('outer-component', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + this.set('person', { name: 'Alex' }); + } + }), + template: `Hi {{person.name}}! {{component "error-component" person=person}}` + }); + + this.registerComponent('error-component', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + this.set('person.name', { name: 'Ben' }); + } + }), + template: '{{person.name}}' + }); + + let expectedBacktrackingMessage = /modified "person\.name" twice on \[object Object\] in a single render\. It was rendered in "component:outer-component" and modified in "component:error-component"/; + + if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + expectDeprecation(expectedBacktrackingMessage); + this.render('{{component componentName}}', { componentName: 'outer-component' }); + } else { + expectAssertion(() => { + this.render('{{component componentName}}', { componentName: 'outer-component' }); + }, expectedBacktrackingMessage); + } + } }); diff --git a/packages/ember-glimmer/tests/integration/helpers/render-test.js b/packages/ember-glimmer/tests/integration/helpers/render-test.js index 664c1190e70..ecfaa949fef 100644 --- a/packages/ember-glimmer/tests/integration/helpers/render-test.js +++ b/packages/ember-glimmer/tests/integration/helpers/render-test.js @@ -1,4 +1,4 @@ -import { observer, set } from 'ember-metal'; +import { observer, set, computed, isFeatureEnabled } from 'ember-metal'; import { Controller } from 'ember-runtime'; import { RenderingTest, moduleFor } from '../../utils/test-case'; @@ -432,4 +432,32 @@ moduleFor('Helpers test: {{render}}', class extends RenderingTest { postController.send('someAction'); } + + ['@test render helper emits useful backtracking re-render assertion message'](assert) { + this.owner.register('controller:outer', Controller.extend()); + this.owner.register('controller:inner', Controller.extend({ + propertyWithError: computed(function() { + this.set('model.name', 'this will cause a backtracking error'); + return 'foo'; + }) + })); + + let expectedBacktrackingMessage = /modified "model\.name" twice on \[object Object\] in a single render\. It was rendered in "controller:outer \(with render helper\)" and modified in "controller:inner \(with render helper\)"/; + + expectDeprecation(() => { + let person = { name: 'Ben' }; + + this.registerTemplate('outer', `Hi {{model.name}} | {{render 'inner' model}}`); + this.registerTemplate('inner', `Hi {{propertyWithError}}`); + + if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + expectDeprecation(expectedBacktrackingMessage); + this.render(`{{render 'outer' person}}`, { person }); + } else { + expectAssertion(() => { + this.render(`{{render 'outer' person}}`, { person }); + }, expectedBacktrackingMessage); + } + }); + } }); diff --git a/packages/ember-glimmer/tests/integration/mount-test.js b/packages/ember-glimmer/tests/integration/mount-test.js index 3dc87aa30e9..f401b5cbb63 100644 --- a/packages/ember-glimmer/tests/integration/mount-test.js +++ b/packages/ember-glimmer/tests/integration/mount-test.js @@ -4,9 +4,9 @@ import { ApplicationTest, RenderingTest } from '../utils/test-case'; -import { compile } from '../utils/helpers'; +import { compile, Component } from '../utils/helpers'; import { Controller } from 'ember-runtime'; -import { set } from 'ember-metal'; +import { set, isFeatureEnabled } from 'ember-metal'; import { Engine, getEngineParent } from 'ember-application'; moduleFor('{{mount}} assertions', class extends RenderingTest { @@ -81,4 +81,39 @@ moduleFor('{{mount}} test', class extends ApplicationTest { this.assertComponentElement(this.firstChild, { content: '

Chat here, dgeb

' }); }); } + + ['@test it emits a useful backtracking re-render assertion message'](assert) { + this.router.map(function() { + this.route('route-with-mount'); + }); + + this.registerTemplate('index', ''); + this.registerTemplate('route-with-mount', '{{mount "chat"}}'); + + this.engineRegistrations['template:application'] = compile('hi {{person.name}} [{{component-with-backtracking-set person=person}}]', { moduleName: 'application' }); + this.engineRegistrations['controller:application'] = Controller.extend({ + person: { name: 'Alex' } + }); + + this.engineRegistrations['template:components/component-with-backtracking-set'] = compile('[component {{person.name}}]', { moduleName: 'components/component-with-backtracking-set' }); + this.engineRegistrations['component:component-with-backtracking-set'] = Component.extend({ + init() { + this._super(...arguments); + this.set('person.name', 'Ben'); + } + }); + + let expectedBacktrackingMessage = /modified "person\.name" twice on \[object Object\] in a single render\. It was rendered in "route-with-mount" \(in the "chat" engine\) and modified in "component:component-with-backtracking-set" \(in the "chat" engine\)/; + + if (isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { + expectDeprecation(expectedBacktrackingMessage); + return this.visit('/route-with-mount'); + } else { + return this.visit('/').then(() => { + expectAssertion(() => { + this.visit('/route-with-mount'); + }, expectedBacktrackingMessage); + }); + } + } }); diff --git a/packages/ember-metal/lib/meta.js b/packages/ember-metal/lib/meta.js index 40d9e9fbac8..4b194985ac3 100644 --- a/packages/ember-metal/lib/meta.js +++ b/packages/ember-metal/lib/meta.js @@ -14,6 +14,7 @@ import { runInDebug, assert } from './debug'; import { removeChainWatcher } from './chains'; +import { has } from 'require'; let counters = { peekCalls: 0, @@ -72,7 +73,10 @@ const IS_PROXY = 1 << 4; if (isEnabled('ember-glimmer-detect-backtracking-rerender') || isEnabled('ember-glimmer-allow-backtracking-rerender')) { members.lastRendered = ownMap; - members.lastRenderedFrom = ownMap; // FIXME: not used in production, remove me from prod builds + if (has('ember-debug')) { //https://github.com/emberjs/ember.js/issues/14732 + members.lastRenderedReferenceMap = ownMap; + members.lastRenderedTemplateMap = ownMap; + } } let memberNames = Object.keys(members); @@ -113,7 +117,10 @@ export function Meta(obj, parentMeta) { if (isEnabled('ember-glimmer-detect-backtracking-rerender') || isEnabled('ember-glimmer-allow-backtracking-rerender')) { this._lastRendered = undefined; - this._lastRenderedFrom = undefined; // FIXME: not used in production, remove me from prod builds + runInDebug(() => { + this._lastRenderedReferenceMap = undefined; + this._lastRenderedTemplateMap = undefined; + }); } this._initializeListeners(); diff --git a/packages/ember-metal/lib/transaction.js b/packages/ember-metal/lib/transaction.js index 5b1e75ca5b0..0733bd86d52 100644 --- a/packages/ember-metal/lib/transaction.js +++ b/packages/ember-metal/lib/transaction.js @@ -23,10 +23,14 @@ if (isEnabled('ember-glimmer-detect-backtracking-rerender') || let counter = 0; let inTransaction = false; let shouldReflush; + let debugStack; runInTransaction = function(context, methodName) { shouldReflush = false; inTransaction = true; + runInDebug(() => { + debugStack = context.env.debugStack; + }); context[methodName](); inTransaction = false; counter++; @@ -40,8 +44,11 @@ if (isEnabled('ember-glimmer-detect-backtracking-rerender') || lastRendered[key] = counter; runInDebug(() => { - let lastRenderedFrom = meta.writableLastRenderedFrom(); - lastRenderedFrom[key] = reference; + let referenceMap = meta.writableLastRenderedReferenceMap(); + referenceMap[key] = reference; + + let templateMap = meta.writableLastRenderedTemplateMap(); + templateMap[key] = debugStack.peek(); }); }; @@ -52,10 +59,13 @@ if (isEnabled('ember-glimmer-detect-backtracking-rerender') || if (lastRendered && lastRendered[key] === counter) { raise( (function() { - let ref = meta.readableLastRenderedFrom(); - let parts = []; - let lastRef = ref[key]; + let templateMap = meta.readableLastRenderedTemplateMap(); + let lastRenderedIn = templateMap[key]; + let currentlyIn = debugStack.peek(); + let referenceMap = meta.readableLastRenderedReferenceMap(); + let lastRef = referenceMap[key]; + let parts = []; let label; if (lastRef) { @@ -64,12 +74,12 @@ if (isEnabled('ember-glimmer-detect-backtracking-rerender') || lastRef = lastRef._parentReference; } - label = parts.join(); + label = parts.join('.'); } else { label = 'the same value'; } - return `You modified ${label} twice on ${object} in a single render. This was unreliable and slow in Ember 1.x and ${implication}`; + return `You modified "${label}" twice on ${object} in a single render. It was rendered in ${lastRenderedIn} and modified in ${currentlyIn}. This was unreliable and slow in Ember 1.x and ${implication}`; }()), false);