Skip to content

Commit

Permalink
[BUGFIX beta] better backtracking re-render assertion
Browse files Browse the repository at this point in the history
  • Loading branch information
GavinJoyce committed Dec 17, 2016
1 parent f1a4a71 commit 9993a34
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 14 deletions.
3 changes: 3 additions & 0 deletions packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
SimpleHelperReference,
ClassBasedHelperReference
} from './utils/references';
import Stack from './utils/stack';

import {
inlineIf,
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions packages/ember-glimmer/lib/syntax/curly-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -302,6 +304,8 @@ class CurlyComponentManager {
didRenderLayout(bucket, bounds) {
bucket.component[BOUNDS] = bounds;
bucket.finalize();

runInDebug(() => this.templateDebugStack.pop());
}

getTag({ component }) {
Expand All @@ -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)) {
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand Down
13 changes: 12 additions & 1 deletion packages/ember-glimmer/lib/syntax/outlet.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -203,19 +205,28 @@ class OutletComponentManager {

didRenderLayout(bucket) {
bucket.finalize();

runInDebug(() => this.templateDebugStack.pop());
}

didCreateElement() {}
didCreate(state) {}
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());
}

Expand Down
17 changes: 17 additions & 0 deletions packages/ember-glimmer/lib/utils/stack.js
Original file line number Diff line number Diff line change
@@ -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];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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 <Ember.Object.+> in a single render/);
expectDeprecation(/rendered "wrapper.content" in "component:x-outer" and modified it in "component:x-inner" \(<Ember.Object.+>\) in a single render/);
}

let outer;
Expand Down Expand Up @@ -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 <Ember.Object.+> in a single render/);
}, /rendered "wrapper.content" in "component:x-outer" and modified it in "component:x-inner" \(<Ember.Object.+>\) in a single render/);

return;
} else {
Expand Down
8 changes: 5 additions & 3 deletions packages/ember-metal/lib/meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
23 changes: 17 additions & 6 deletions packages/ember-metal/lib/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand All @@ -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();
});
};

Expand All @@ -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) {
Expand All @@ -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);

Expand Down

0 comments on commit 9993a34

Please sign in to comment.