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 Jan 6, 2017
1 parent 9b3d581 commit cd11371
Show file tree
Hide file tree
Showing 14 changed files with 344 additions and 32 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 DebugStack from './utils/debug-stack';

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

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

getTag({ component }) {
Expand All @@ -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)) {
Expand All @@ -348,6 +354,8 @@ class CurlyComponentManager {

didUpdateLayout(bucket) {
bucket.finalize();

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

didUpdate({ component, environment }) {
Expand All @@ -362,12 +370,21 @@ class CurlyComponentManager {
}
}

runInDebug(() => {
CurlyComponentManager.prototype._pushToDebugStack = function(name, environment) {
this.debugStack = environment.debugStack;
this.debugStack.push(name);
};
});

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;
Expand Down
17 changes: 15 additions & 2 deletions packages/ember-glimmer/lib/syntax/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -74,6 +74,8 @@ class MountManager {
}

create(environment, { name, env }, args, dynamicScope) {
runInDebug(() => this._pushToDebugStack(`engine:${name}`, env));

dynamicScope.outletState = UNDEFINED_REFERENCE;

let engine = env.owner.buildChildEngineInstance(name);
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 14 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._pushToDebugStack(`template:${definition.template.meta.moduleName}`, environment));

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

didRenderLayout(bucket) {
bucket.finalize();

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

didCreateElement() {}
Expand All @@ -212,10 +216,19 @@ class OutletComponentManager {
didUpdate(state) {}
}

runInDebug(() => {
OutletComponentManager.prototype._pushToDebugStack = function(name, environment) {
this.debugStack = environment.debugStack;
this.debugStack.push(name);
};
});

const MANAGER = new OutletComponentManager();

class TopLevelOutletComponentManager extends OutletComponentManager {
create(environment, definition, args, dynamicScope) {
runInDebug(() => this._pushToDebugStack(`template:${definition.template.meta.moduleName}`, environment));

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

Expand Down
17 changes: 16 additions & 1 deletion packages/ember-glimmer/lib/syntax/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -168,11 +168,24 @@ class AbstractRenderManager {
didUpdate() {}
}

runInDebug(() => {
AbstractRenderManager.prototype.didRenderLayout = function() {
this.debugStack.pop();
};

AbstractRenderManager.prototype._pushToDebugStack = function(name, environment) {
this.debugStack = environment.debugStack;
this.debugStack.push(name);
};
});

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(`controller:${name} (with the render helper)`, environment));

if (dynamicScope.rootOutletState) {
dynamicScope.outletState = dynamicScope.rootOutletState.getOrphan(name);
}
Expand All @@ -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(`controller:${name} (with the render helper)`, environment));

if (dynamicScope.rootOutletState) {
dynamicScope.outletState = dynamicScope.rootOutletState.getOrphan(name);
}
Expand Down
66 changes: 66 additions & 0 deletions packages/ember-glimmer/lib/utils/debug-stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
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));
}

pushEngine(name) {
this._stack.push(new EngineElement(name));
}

pop() {
let element = this._stack.pop();

if (element) {
return element.name;
}
}

peek() {
let template = this._currentTemplate();
let engine = this._currentEngine();

if (engine) {
return `"${template}" (in "${engine}")`;
} else if (template) {
return `"${template}"`;
}
}

_currentTemplate() {
return this._getCurrentByType(TemplateElement);
}

_currentEngine() {
return this._getCurrentByType(EngineElement);
}

_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;
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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 "template: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);
});
}
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -2089,10 +2089,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', {
Expand Down Expand Up @@ -2137,14 +2133,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');
Expand All @@ -2162,10 +2161,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 <Ember.Object.+> in a single render/);
}

let outer;

this.registerComponent('x-outer', {
Expand All @@ -2190,14 +2185,17 @@ moduleFor('Components test: curly components', class extends RenderingTest {
template: '<div id="inner-value">{{wrapper.content}}</div>'
});

if (!isFeatureEnabled('ember-glimmer-allow-backtracking-rerender')) {
let expectedBacktrackingMessage = /modified "wrapper\.content" twice on <Ember\.Object.+> 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 <Ember.Object.+> in a single render/);
}, expectedBacktrackingMessage);

return;
} else {
this.render('{{x-outer}}');
}

assert.equal(this.$('#inner-value').text(), '1', 'initial render of inner');
Expand Down
Loading

0 comments on commit cd11371

Please sign in to comment.