Skip to content

Commit

Permalink
[wip] better backtracking re-render assertion
Browse files Browse the repository at this point in the history
  • Loading branch information
GavinJoyce committed Dec 16, 2016
1 parent 336f7dc commit 1e845d5
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 6 deletions.
21 changes: 21 additions & 0 deletions packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,25 @@ const builtInComponents = {

import { default as ActionModifierManager } from './modifiers/action';

//TODO: GJ: extract
class TemplateDebugStack {
constructor() {
this._stack = [];
}

push(templateName) {
this._stack.push(templateName);
}

pop() {
return this._stack.pop();
}

getCurrent() {
return this._stack[this._stack.length - 1];
}
}

export default class Environment extends GlimmerEnvironment {
static create(options) {
return new Environment(options);
Expand Down Expand Up @@ -131,6 +150,8 @@ export default class Environment extends GlimmerEnvironment {
'-html-safe': htmlSafeHelper,
'-get-dynamic-var': getDynamicVar
};

runInDebug(() => this.templateDebugStack = new TemplateDebugStack());
}

// Hello future traveler, welcome to the world of syntax refinement.
Expand Down
20 changes: 20 additions & 0 deletions packages/ember-glimmer/lib/syntax/curly-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ class CurlyComponentManager {
}

create(environment, definition, args, dynamicScope, callerSelfRef, hasBlock) {
runInDebug(() => {
let templateName = definition.ComponentClass._debugContainerKey;
environment.templateDebugStack.push(templateName);
this.templateDebugStack = environment.templateDebugStack;
});

let parentView = dynamicScope.view;

let klass = definition.ComponentClass;
Expand Down Expand Up @@ -302,6 +308,8 @@ class CurlyComponentManager {
didRenderLayout(bucket, bounds) {
bucket.component[BOUNDS] = bounds;
bucket.finalize();

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

getTag({ component }) {
Expand All @@ -319,6 +327,12 @@ class CurlyComponentManager {
update(bucket, _, dynamicScope) {
let { component, args, argsRevision, environment } = bucket;

runInDebug(() => {
let templateName = Object.getPrototypeOf(bucket.component)._debugContainerKey;
environment.templateDebugStack.push(templateName);
this.templateDebugStack = environment.templateDebugStack; //TODO: GJ: do we need this?
});

bucket.finalizer = _instrumentStart('render.component', rerenderInstrumentDetails, component);

if (!args.tag.validate(argsRevision)) {
Expand Down Expand Up @@ -363,6 +377,12 @@ const MANAGER = new CurlyComponentManager();

class TopComponentManager extends CurlyComponentManager {
create(environment, definition, args, dynamicScope, currentScope, hasBlock) {
runInDebug(() => {
let templateName = definition.ComponentClass._debugContainerKey;
environment.templateDebugStack.push(templateName);
this.templateDebugStack = environment.templateDebugStack;
});

let component = definition.ComponentClass;

let finalizer = _instrumentStart('render.component', initialRenderInstrumentDetails, component);
Expand Down
14 changes: 13 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,11 @@ class OutletComponentManager {
}

create(environment, definition, args, dynamicScope) {
runInDebug(() => {
environment.templateDebugStack.push(definition.template.meta.moduleName);
this.templateDebugStack = environment.templateDebugStack;
});

let outletStateReference = dynamicScope.outletState = dynamicScope.outletState.get('outlets').get(definition.outletName);
let outletState = outletStateReference.value();
return new StateBucket(outletState);
Expand All @@ -203,6 +208,8 @@ class OutletComponentManager {

didRenderLayout(bucket) {
bucket.finalize();

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

didCreateElement() {}
Expand All @@ -216,6 +223,11 @@ const MANAGER = new OutletComponentManager();

class TopLevelOutletComponentManager extends OutletComponentManager {
create(environment, definition, args, dynamicScope) {
runInDebug(() => {
environment.templateDebugStack.push(definition.template.meta.moduleName);
this.templateDebugStack = environment.templateDebugStack;
});

return new StateBucket(dynamicScope.outletState.value());
}

Expand Down
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
3 changes: 3 additions & 0 deletions packages/ember-metal/lib/meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ const IS_PROXY = 1 << 4;
if (isEnabled('ember-glimmer-detect-backtracking-rerender') ||
isEnabled('ember-glimmer-allow-backtracking-rerender')) {
members.lastRendered = ownMap;
//TODO: GJ: are these the best names?
members.lastRenderedFrom = ownMap; // FIXME: not used in production, remove me from prod builds
members.lastRenderedInTemplate = ownMap;
}

let memberNames = Object.keys(members);
Expand Down Expand Up @@ -109,6 +111,7 @@ export function Meta(obj, parentMeta) {
isEnabled('ember-glimmer-allow-backtracking-rerender')) {
this._lastRendered = undefined;
this._lastRenderedFrom = undefined; // FIXME: not used in production, remove me from prod builds
this._lastRenderedInTemplate = undefined;
}

this._initializeListeners();
Expand Down
15 changes: 14 additions & 1 deletion 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 @@ -42,6 +46,11 @@ if (isEnabled('ember-glimmer-detect-backtracking-rerender') ||
runInDebug(() => {
let lastRenderedFrom = meta.writableLastRenderedFrom();
lastRenderedFrom[key] = reference;

if (templateDebugStack) { //TODO: GJ: why is templateDebugStack sometimes undefined?
var lastRenderedInTemplate = meta.writableLastRenderedInTemplate();
lastRenderedInTemplate[key] = templateDebugStack.getCurrent();
}
});
};

Expand Down Expand Up @@ -69,7 +78,11 @@ 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}`;
let templates = meta.readableLastRenderedInTemplate();
var lastRenderedInTemplate = templates[key];

let currentTemplate = templateDebugStack.getCurrent();
return `You rendered "${parts.join('.')}" in "${lastRenderedInTemplate}" 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 1e845d5

Please sign in to comment.