From 9993a3405aceec1b60928526f52261e8c79b1c8a Mon Sep 17 00:00:00 2001 From: Gavin Joyce Date: Thu, 15 Dec 2016 20:35:49 +0000 Subject: [PATCH] [BUGFIX beta] better backtracking re-render assertion --- packages/ember-glimmer/lib/environment.js | 3 +++ .../lib/syntax/curly-component.js | 14 +++++++++++ packages/ember-glimmer/lib/syntax/outlet.js | 13 ++++++++++- packages/ember-glimmer/lib/utils/stack.js | 17 ++++++++++++++ .../components/curly-components-test.js | 8 +++---- packages/ember-metal/lib/meta.js | 8 ++++--- packages/ember-metal/lib/transaction.js | 23 ++++++++++++++----- 7 files changed, 72 insertions(+), 14 deletions(-) create mode 100644 packages/ember-glimmer/lib/utils/stack.js diff --git a/packages/ember-glimmer/lib/environment.js b/packages/ember-glimmer/lib/environment.js index a0a8d050871..46aba581e32 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 Stack from './utils/stack'; import { inlineIf, @@ -131,6 +132,8 @@ export default class Environment extends GlimmerEnvironment { '-html-safe': htmlSafeHelper, '-get-dynamic-var': getDynamicVar }; + + runInDebug(() => this.templateDebugStack = new Stack()); } // 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 b780a5d279f..2fa9694573f 100644 --- a/packages/ember-glimmer/lib/syntax/curly-component.js +++ b/packages/ember-glimmer/lib/syntax/curly-component.js @@ -190,6 +190,8 @@ class CurlyComponentManager { let processedArgs = ComponentArgs.create(args); let { props } = processedArgs.value(); + runInDebug(() => this._pushTemplateToDebugStack(klass, environment)); + aliasIdToElementId(args, props); props.parentView = parentView; @@ -302,6 +304,8 @@ class CurlyComponentManager { didRenderLayout(bucket, bounds) { bucket.component[BOUNDS] = bounds; bucket.finalize(); + + runInDebug(() => this.templateDebugStack.pop()); } getTag({ component }) { @@ -319,6 +323,8 @@ class CurlyComponentManager { update(bucket, _, dynamicScope) { let { component, args, argsRevision, environment } = bucket; + runInDebug(() => this._pushTemplateToDebugStack(Object.getPrototypeOf(component), environment)); + bucket.finalizer = _instrumentStart('render.component', rerenderInstrumentDetails, component); if (!args.tag.validate(argsRevision)) { @@ -357,6 +363,12 @@ class CurlyComponentManager { getDestructor(stateBucket) { return stateBucket; } + + _pushTemplateToDebugStack(componentClass, environment) { + let templateName = componentClass._debugContainerKey; + environment.templateDebugStack.push(templateName); + this.templateDebugStack = environment.templateDebugStack; + } } const MANAGER = new CurlyComponentManager(); @@ -365,6 +377,8 @@ class TopComponentManager extends CurlyComponentManager { create(environment, definition, args, dynamicScope, currentScope, hasBlock) { let component = definition.ComponentClass; + runInDebug(() => this._pushTemplateToDebugStack(component, environment)); + let finalizer = _instrumentStart('render.component', initialRenderInstrumentDetails, component); dynamicScope.view = component; diff --git a/packages/ember-glimmer/lib/syntax/outlet.js b/packages/ember-glimmer/lib/syntax/outlet.js index f20455ca7de..3f6bd2408d5 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._pushTemplateToDebugStack(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.templateDebugStack.pop()); } didCreateElement() {} @@ -210,12 +214,19 @@ class OutletComponentManager { update(bucket) {} didUpdateLayout(bucket) {} didUpdate(state) {} + + _pushTemplateToDebugStack(definition, environment) { + environment.templateDebugStack.push(definition.template.meta.moduleName); + this.templateDebugStack = environment.templateDebugStack; + } } const MANAGER = new OutletComponentManager(); class TopLevelOutletComponentManager extends OutletComponentManager { create(environment, definition, args, dynamicScope) { + runInDebug(() => this._pushTemplateToDebugStack(definition, environment)); + return new StateBucket(dynamicScope.outletState.value()); } diff --git a/packages/ember-glimmer/lib/utils/stack.js b/packages/ember-glimmer/lib/utils/stack.js new file mode 100644 index 00000000000..a9a64dd1e46 --- /dev/null +++ b/packages/ember-glimmer/lib/utils/stack.js @@ -0,0 +1,17 @@ +export default class Stack { + constructor() { + this._stack = []; + } + + push(item) { + this._stack.push(item); + } + + pop() { + return this._stack.pop(); + } + + peek() { + return this._stack[this._stack.length - 1]; + } +} 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..e5dabee1fbf 100644 --- a/packages/ember-glimmer/tests/integration/components/curly-components-test.js +++ b/packages/ember-glimmer/tests/integration/components/curly-components-test.js @@ -2076,7 +2076,7 @@ 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/); + expectDeprecation(/rendered "value" in "component:x-middle" and modified it in "component:x-inner" \(<\(.+>\) in a single render/); } let outer, middle; @@ -2126,7 +2126,7 @@ moduleFor('Components test: curly components', class extends RenderingTest { if (!isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { expectAssertion(() => { this.runTask(() => outer.set('value', 2)); - }, /modified value twice on <\(.+> in a single render/); + }, /rendered "value" in "component:x-middle" and modified it in "component:x-inner" \(<\(.+>\) in a single render/); return; } else { @@ -2149,7 +2149,7 @@ 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/); + expectDeprecation(/rendered "wrapper.content" in "component:x-outer" and modified it in "component:x-inner" \(\) in a single render/); } let outer; @@ -2179,7 +2179,7 @@ moduleFor('Components test: curly components', class extends RenderingTest { if (!isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) { expectAssertion(() => { this.render('{{x-outer}}'); - }, /modified wrapper.content twice on in a single render/); + }, /rendered "wrapper.content" in "component:x-outer" and modified it in "component:x-inner" \(\) in a single render/); return; } else { diff --git a/packages/ember-metal/lib/meta.js b/packages/ember-metal/lib/meta.js index f76ecdbcfd8..ce71c980bde 100644 --- a/packages/ember-metal/lib/meta.js +++ b/packages/ember-metal/lib/meta.js @@ -67,7 +67,8 @@ 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 + members.lastRenderedReferenceMap = ownMap; // FIXME: not used in production, remove me from prod builds + members.lastRenderedTemplateMap = ownMap; // FIXME: not used in production, remove me from prod builds } let memberNames = Object.keys(members); @@ -107,8 +108,9 @@ 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 + this.lastRendered = undefined; + this.lastRenderedReferenceMap = undefined; // FIXME: not used in production, remove me from prod builds + this.lastRenderedTemplateMap = undefined; // FIXME: not used in production, remove me from prod builds } this._initializeListeners(); diff --git a/packages/ember-metal/lib/transaction.js b/packages/ember-metal/lib/transaction.js index 5b1e75ca5b0..efe52265a3e 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 templateDebugStack; runInTransaction = function(context, methodName) { shouldReflush = false; inTransaction = true; + runInDebug(() => { + templateDebugStack = context.env.templateDebugStack; + }); 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] = templateDebugStack.peek(); }); }; @@ -52,10 +59,14 @@ 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 lastRenderedTemplate = templateMap[key]; + + let currentTemplate = templateDebugStack.peek(); + let referenceMap = meta.readableLastRenderedReferenceMap(); + let lastRef = referenceMap[key]; + let parts = []; let label; if (lastRef) { @@ -69,7 +80,7 @@ if (isEnabled('ember-glimmer-detect-backtracking-rerender') || 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 rendered "${parts.join('.')}" in "${lastRenderedTemplate}" and modified it in "${currentTemplate}" (${object}) in a single render. This was unreliable and slow in Ember 1.x and ${implication}`; }()), false);