Skip to content

Commit

Permalink
Merge pull request #16185 from emberjs/fix-did-receive-attrs-in-es-class
Browse files Browse the repository at this point in the history 
Fix didReceiveAttrs in ES Class
  • Loading branch information
@mmun
mmun authored Feb 17, 2018
2 parents ccce19b + 2cd6e2f commit 4a4c9cd
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 126 deletions.
1 change: 1 addition & 0 deletions packages/ember-environment/lib/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const environment: {
}

export const ENV: {
_ENABLE_DID_INIT_ATTRS_SUPPORT: boolean;
_APPLICATION_TEMPLATE_WRAPPER: boolean;
_ENABLE_RENDER_SUPPORT: boolean;
_TEMPLATE_ONLY_GLIMMER_COMPONENTS: boolean;
Expand Down
7 changes: 7 additions & 0 deletions packages/ember-glimmer/lib/component-managers/curly.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
deprecate,
} from 'ember-debug';
import { DEBUG } from 'ember-env-flags';
import { ENV } from 'ember-environment';
import {
_instrumentStart, get,
} from 'ember-metal';
Expand Down Expand Up @@ -231,6 +232,12 @@ export default class CurlyComponentManager extends AbstractManager<ComponentStat
parentView.appendChild(component);
}

if (ENV._ENABLE_DID_INIT_ATTRS_SUPPORT === true) {
component.trigger('didInitAttrs');
}

component.trigger('didReceiveAttrs');

// We usually do this in the `didCreateElement`, but that hook doesn't fire for tagless components
if (component.tagName === '') {
if (environment.isInteractive) {
Expand Down
3 changes: 1 addition & 2 deletions packages/ember-glimmer/tests/integration/components/append-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ class AbstractAppendTest extends RenderingTest {

assert.deepEqual(hooks, [
['x-parent', 'init'],
['x-parent', 'didReceiveAttrs'],
['x-parent', 'on(init)']
], 'creation of x-parent');

Expand All @@ -150,8 +149,8 @@ class AbstractAppendTest extends RenderingTest {
['x-parent', 'willInsertElement'],

['x-child', 'init'],
['x-child', 'didReceiveAttrs'],
['x-child', 'on(init)'],
['x-child', 'didReceiveAttrs'],
['x-child', 'willRender'],
['x-child', 'willInsertElement'],

Expand Down
57 changes: 21 additions & 36 deletions packages/ember-glimmer/tests/integration/components/curly-components-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2777,39 +2777,6 @@ moduleFor('Components test: curly components', class extends RenderingTest {
}, /didInitAttrs called/);
}

// This test is a replication of the "component unit tests" scenario. When we deprecate
// and remove them, this test could be removed as well. This is not fully/intentionally
// supported, and it is unclear that this particular behavior is actually relied on.
// Since there is no real "invocation" here, it has other issues and inconsistencies,
// like there is no real "attrs" here, and there is no "update" pass.
['@test didReceiveAttrs fires even if component is not rendered'](assert) {
let didReceiveAttrsCount = 0;

this.registerComponent('foo-bar', {
ComponentClass: Component.extend({
init() {
this._super(...arguments);
this.didInit = true;
},

didReceiveAttrs() {
assert.ok(this.didInit, 'expected init to have run before didReceiveAttrs');
didReceiveAttrsCount++;
},

willRender() {
throw new Error('Unexpected render!');
}
})
});

assert.strictEqual(didReceiveAttrsCount, 0, 'precond: didReceiveAttrs is not fired');

this.runTask(() => this.component = this.owner.lookup('component:foo-bar'));

assert.strictEqual(didReceiveAttrsCount, 1, 'precond: didReceiveAttrs is fired');
}

['@test didReceiveAttrs fires after .init() but before observers become active'](assert) {
let barCopyDidChangeCount = 0;

Expand All @@ -2835,13 +2802,13 @@ moduleFor('Components test: curly components', class extends RenderingTest {

this.assertText('3-4');

assert.strictEqual(barCopyDidChangeCount, 0, 'expected NO observer firing for: barCopy');
assert.strictEqual(barCopyDidChangeCount, 1, 'expected observer firing for: barCopy');

this.runTask(() => set(this.context, 'bar', 7));

this.assertText('7-8');

assert.strictEqual(barCopyDidChangeCount, 1, 'expected observer firing for: barCopy');
assert.strictEqual(barCopyDidChangeCount, 2, 'expected observer firing for: barCopy');
}

['@test overriding didReceiveAttrs does not trigger deprecation'](assert) {
Expand Down Expand Up @@ -3120,6 +3087,24 @@ moduleFor('Components test: curly components', class extends RenderingTest {

this.assertText('3');
}

['@test has attrs by didReceiveAttrs with native classes'](assert) {
class FooBarComponent extends Component {
constructor(injections) {
super(injections);
// analagous to class field defaults
this.foo = 'bar';
}

didReceiveAttrs() {
assert.equal(this.foo, 'bar', 'received default attrs correctly');
}
}

this.registerComponent('foo-bar', { ComponentClass: FooBarComponent });

this.render('{{foo-bar}}');
}
});

if (jQueryDisabled) {
Expand Down Expand Up @@ -3200,4 +3185,4 @@ if (jQueryDisabled) {
}

});
}
}
62 changes: 31 additions & 31 deletions packages/ember-glimmer/tests/integration/components/life-cycle-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,20 +295,20 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

['the-top', 'init'],
['the-top', 'didReceiveAttrs'],
['the-top', 'on(init)'],
['the-top', 'didReceiveAttrs'],
['the-top', 'willRender'],
['the-top', 'willInsertElement'],

['the-middle', 'init'],
['the-middle', 'didReceiveAttrs'],
['the-middle', 'on(init)'],
['the-middle', 'didReceiveAttrs'],
['the-middle', 'willRender'],
['the-middle', 'willInsertElement'],

['the-bottom', 'init'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'willRender'],
['the-bottom', 'willInsertElement'],

Expand All @@ -328,16 +328,16 @@ class LifeCycleHooksTest extends RenderingTest {
nonInteractive: [
// Sync hooks
['the-top', 'init'],
['the-top', 'didReceiveAttrs'],
['the-top', 'on(init)'],
['the-top', 'didReceiveAttrs'],

['the-middle', 'init'],
['the-middle', 'didReceiveAttrs'],
['the-middle', 'on(init)'],
['the-middle', 'didReceiveAttrs'],

['the-bottom', 'init'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)']
['the-bottom', 'on(init)'],
['the-bottom', 'didReceiveAttrs']
]
});

Expand Down Expand Up @@ -526,27 +526,27 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

['the-parent', 'init'],
['the-parent', 'didReceiveAttrs'],
['the-parent', 'on(init)'],
['the-parent', 'didReceiveAttrs'],
['the-parent', 'willRender'],
['the-parent', 'willInsertElement'],


['the-first-child', 'init'],
['the-first-child', 'didReceiveAttrs'],
['the-first-child', 'on(init)'],
['the-first-child', 'didReceiveAttrs'],
['the-first-child', 'willRender'],
['the-first-child', 'willInsertElement'],

['the-second-child', 'init'],
['the-second-child', 'didReceiveAttrs'],
['the-second-child', 'on(init)'],
['the-second-child', 'didReceiveAttrs'],
['the-second-child', 'willRender'],
['the-second-child', 'willInsertElement'],

['the-last-child', 'init'],
['the-last-child', 'didReceiveAttrs'],
['the-last-child', 'on(init)'],
['the-last-child', 'didReceiveAttrs'],
['the-last-child', 'willRender'],
['the-last-child', 'willInsertElement'],

Expand All @@ -569,20 +569,20 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

['the-parent', 'init'],
['the-parent', 'didReceiveAttrs'],
['the-parent', 'on(init)'],
['the-parent', 'didReceiveAttrs'],

['the-first-child', 'init'],
['the-first-child', 'didReceiveAttrs'],
['the-first-child', 'on(init)'],
['the-first-child', 'didReceiveAttrs'],

['the-second-child', 'init'],
['the-second-child', 'didReceiveAttrs'],
['the-second-child', 'on(init)'],
['the-second-child', 'didReceiveAttrs'],

['the-last-child', 'init'],
['the-last-child', 'didReceiveAttrs'],
['the-last-child', 'on(init)']
['the-last-child', 'on(init)'],
['the-last-child', 'didReceiveAttrs']
]
});

Expand Down Expand Up @@ -832,20 +832,20 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

['the-top', 'init'],
['the-top', 'didReceiveAttrs'],
['the-top', 'on(init)'],
['the-top', 'didReceiveAttrs'],
['the-top', 'willRender'],
['the-top', 'willInsertElement'],

['the-middle', 'init'],
['the-middle', 'didReceiveAttrs'],
['the-middle', 'on(init)'],
['the-middle', 'didReceiveAttrs'],
['the-middle', 'willRender'],
['the-middle', 'willInsertElement'],

['the-bottom', 'init'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'willRender'],
['the-bottom', 'willInsertElement'],

Expand All @@ -865,16 +865,16 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

['the-top', 'init'],
['the-top', 'didReceiveAttrs'],
['the-top', 'on(init)'],
['the-top', 'didReceiveAttrs'],

['the-middle', 'init'],
['the-middle', 'didReceiveAttrs'],
['the-middle', 'on(init)'],
['the-middle', 'didReceiveAttrs'],

['the-bottom', 'init'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)']
['the-bottom', 'on(init)'],
['the-bottom', 'didReceiveAttrs']
]
});

Expand Down Expand Up @@ -1006,8 +1006,8 @@ class LifeCycleHooksTest extends RenderingTest {
let initialHooks = () => {
let ret = [
['an-item', 'init'],
['an-item', 'didReceiveAttrs'],
['an-item', 'on(init)']
['an-item', 'on(init)'],
['an-item', 'didReceiveAttrs']
];
if (this.isInteractive) {
ret.push(
Expand All @@ -1017,8 +1017,8 @@ class LifeCycleHooksTest extends RenderingTest {
}
ret.push(
['nested-item', 'init'],
['nested-item', 'didReceiveAttrs'],
['nested-item', 'on(init)']
['nested-item', 'on(init)'],
['nested-item', 'didReceiveAttrs']
);
if (this.isInteractive) {
ret.push(
Expand Down Expand Up @@ -1118,15 +1118,15 @@ class LifeCycleHooksTest extends RenderingTest {
['nested-item', 'willClearRender'],

['no-items', 'init'],
['no-items', 'didReceiveAttrs'],
['no-items', 'on(init)'],
['no-items', 'didReceiveAttrs'],
['no-items', 'willRender'],
['no-items', 'willInsertElement'],


['nested-item', 'init'],
['nested-item', 'didReceiveAttrs'],
['nested-item', 'on(init)'],
['nested-item', 'didReceiveAttrs'],
['nested-item', 'willRender'],
['nested-item', 'willInsertElement'],

Expand Down Expand Up @@ -1160,12 +1160,12 @@ class LifeCycleHooksTest extends RenderingTest {

nonInteractive: [
['no-items', 'init'],
['no-items', 'didReceiveAttrs'],
['no-items', 'on(init)'],
['no-items', 'didReceiveAttrs'],

['nested-item', 'init'],
['nested-item', 'didReceiveAttrs'],
['nested-item', 'on(init)'],
['nested-item', 'didReceiveAttrs'],

['an-item', 'willDestroy'],
['nested-item', 'willDestroy'],
Expand Down
Empty file added packages/ember-metal/tests/classes_test.js
View file
Empty file.
Empty file added packages/ember-metal/tests/support.js
View file
Empty file.
7 changes: 0 additions & 7 deletions packages/ember-runtime/lib/system/core_object.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
generateGuid,
makeArray,
GUID_KEY_PROPERTY,
symbol,
NAME_KEY,
GUID_KEY,
HAS_NATIVE_PROXY
Expand Down Expand Up @@ -46,8 +45,6 @@ const schedule = run.schedule;
const applyMixin = Mixin._apply;
const reopen = Mixin.prototype.reopen;

export const POST_INIT = symbol('POST_INIT');

function makeCtor() {
// Note: avoid accessing any properties on the object since it makes the
// method a lot faster. This is glue code so we want it to be as fast as
Expand Down Expand Up @@ -206,8 +203,6 @@ function makeCtor() {
}
self.init(...arguments);

self[POST_INIT]();

m.proto = proto;
finishChains(m);
sendEvent(self, 'init', undefined, undefined, undefined, m);
Expand Down Expand Up @@ -317,8 +312,6 @@ CoreObject.PrototypeMixin = Mixin.create({
*/
init() {},

[POST_INIT]() { }, // Private, and only for didReceiveAttrs

__defineNonEnumerable(property) {
Object.defineProperty(this, property.name, property.descriptor);
//this[property.name] = property.descriptor.value;
Expand Down
Loading

0 comments on commit 4a4c9cd

Please sign in to comment.