Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved backtracking re-render assertion message #14723

Merged
merged 1 commit into from
Jan 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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