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.

Sign up for GitHub

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

[CLEANUP] Remove didInitAttrs lifecycle method #15923

Merged
merged 2 commits into from
Dec 5, 2017
Merged

[CLEANUP] Remove didInitAttrs lifecycle method #15923

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67065ac
[CLEANUP] Remove didInitAttrs lifecycle method
thoov Dec 5, 2017
272cd34
Guard didInitAttrs trigger
thoov Dec 5, 2017
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
8 changes: 1 addition & 7 deletions packages/ember-glimmer/tests/integration/components/append-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class AbstractAppendTest extends RenderingTest {
let options = {
ComponentClass: _options.ComponentClass.extend({
init() {
expectDeprecation(() => { this._super(...arguments); }, /didInitAttrs called/);
this._super(...arguments);
if (name in componentsByName) {
throw new TypeError('Component named: ` ' + name + ' ` already registered');
}
Expand All @@ -59,10 +59,6 @@ class AbstractAppendTest extends RenderingTest {
this.on('init', () => pushHook('on(init)'));
},

didInitAttrs(options) {
pushHook('didInitAttrs', options);
},

didReceiveAttrs() {
pushHook('didReceiveAttrs');
},
Expand Down Expand Up @@ -142,7 +138,6 @@ class AbstractAppendTest extends RenderingTest {

assert.deepEqual(hooks, [
['x-parent', 'init'],
['x-parent', 'didInitAttrs'],
['x-parent', 'didReceiveAttrs'],
['x-parent', 'on(init)']
], 'creation of x-parent');
Expand All @@ -155,7 +150,6 @@ class AbstractAppendTest extends RenderingTest {
['x-parent', 'willInsertElement'],

['x-child', 'init'],
['x-child', 'didInitAttrs'],
['x-child', 'didReceiveAttrs'],
['x-child', 'on(init)'],
['x-child', 'willRender'],
Expand Down
66 changes: 32 additions & 34 deletions packages/ember-glimmer/tests/integration/components/curly-components-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
inject,
Service
} from 'ember-runtime';
import { ENV } from 'ember-environment';
import { Component, compile, htmlSafe } from '../../utils/helpers';
import { strip } from '../../utils/abstract-test-case';
import { moduleFor, RenderingTest } from '../../utils/test-case';
Expand All @@ -27,6 +28,15 @@ import {
} from 'ember/features';

moduleFor('Components test: curly components', class extends RenderingTest {
constructor() {
super();
this.originalDidInitAttrsSupport = ENV._ENABLE_DID_INIT_ATTRS_SUPPORT;
}

teardown() {
ENV._ENABLE_DID_INIT_ATTRS_SUPPORT = this.originalDidInitAttrsSupport;
super.teardown();
}

['@test it can render a basic component']() {
this.registerComponent('foo-bar', { template: 'hello' });
Expand Down Expand Up @@ -2690,6 +2700,8 @@ moduleFor('Components test: curly components', class extends RenderingTest {
}

['@test using didInitAttrs as an event is deprecated'](assert) {
ENV._ENABLE_DID_INIT_ATTRS_SUPPORT = true;

this.registerComponent('foo-bar', {
ComponentClass: Component.extend({
foo: on('didInitAttrs', function() {
Expand All @@ -2703,15 +2715,26 @@ moduleFor('Components test: curly components', class extends RenderingTest {
}, /didInitAttrs called/);
}

['@test using didInitAttrs as an event throws an assert'](assert) {
this.registerComponent('foo-bar', {
ComponentClass: Component.extend({
foo: on('didInitAttrs', function() {
assert.ok(true, 'should fire `didInitAttrs` event');
})
})
});

expectAssertion(() => {
this.render('{{foo-bar}}');
}, /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 did{Init,Receive}Attrs fires even if component is not rendered'](assert) {
expectDeprecation(/didInitAttrs called/);

let didInitAttrsCount = 0;
['@test didReceiveAttrs fires even if component is not rendered'](assert) {
let didReceiveAttrsCount = 0;

this.registerComponent('foo-bar', {
Expand All @@ -2721,11 +2744,6 @@ moduleFor('Components test: curly components', class extends RenderingTest {
this.didInit = true;
},

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

didReceiveAttrs() {
assert.ok(this.didInit, 'expected init to have run before didReceiveAttrs');
didReceiveAttrsCount++;
Expand All @@ -2737,19 +2755,14 @@ moduleFor('Components test: curly components', class extends RenderingTest {
})
});

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

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

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

['@test did{Init,Receive}Attrs fires after .init() but before observers become active'](assert) {
expectDeprecation(/didInitAttrs called/);

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

this.registerComponent('foo-bar', {
Expand All @@ -2759,42 +2772,27 @@ moduleFor('Components test: curly components', class extends RenderingTest {
this.didInit = true;
},

didInitAttrs() {
assert.ok(this.didInit, 'expected init to have run before didInitAttrs');
this.set('fooCopy', this.attrs.foo.value + 1);
},

didReceiveAttrs() {
assert.ok(this.didInit, 'expected init to have run before didReceiveAttrs');
this.set('barCopy', this.attrs.bar.value + 1);
},

fooCopyDidChange: observer('fooCopy', () => { fooCopyDidChangeCount++; }),
barCopyDidChange: observer('barCopy', () => { barCopyDidChangeCount++; })
}),

template: '{{foo}}-{{fooCopy}}-{{bar}}-{{barCopy}}'
template: '{{bar}}-{{barCopy}}'
});

this.render(`{{foo-bar foo=foo bar=bar}}`, { foo: 1, bar: 3 });

this.assertText('1-2-3-4');

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

this.runTask(() => set(this.context, 'foo', 5));
this.render(`{{foo-bar bar=bar}}`, { bar: 3 });

this.assertText('5-2-3-4');
this.assertText('3-4');

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

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

this.assertText('5-2-7-8');
this.assertText('7-8');

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

Expand Down
36 changes: 1 addition & 35 deletions packages/ember-glimmer/tests/integration/components/life-cycle-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ class LifeCycleHooksTest extends RenderingTest {

let ComponentClass = this.ComponentClass.extend({
init() {
expectDeprecation(() => { this._super(...arguments); },
/didInitAttrs called/);
this._super(...arguments);

this.isInitialRender = true;
this.componentName = name;
Expand All @@ -145,13 +144,6 @@ class LifeCycleHooksTest extends RenderingTest {
});
},

didInitAttrs(options) {
pushHook('didInitAttrs', options);
assertParentView('didInitAttrs', this);
assertNoElement('didInitAttrs', this);
assertState('didInitAttrs', 'preRender', this);
},

didReceiveAttrs(options) {
pushHook('didReceiveAttrs', options);
assertParentView('didReceiveAttrs', this);
Expand Down Expand Up @@ -307,21 +299,18 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

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

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

['the-bottom', 'init'],
['the-bottom', 'didInitAttrs'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)'],
['the-bottom', 'willRender'],
Expand All @@ -343,17 +332,14 @@ class LifeCycleHooksTest extends RenderingTest {
nonInteractive: [
// Sync hooks
['the-top', 'init'],
['the-top', 'didInitAttrs'],
['the-top', 'didReceiveAttrs'],
['the-top', 'on(init)'],

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

['the-bottom', 'init'],
['the-bottom', 'didInitAttrs'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)']
]
Expand Down Expand Up @@ -544,29 +530,25 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

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


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

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

['the-last-child', 'init'],
['the-last-child', 'didInitAttrs'],
['the-last-child', 'didReceiveAttrs'],
['the-last-child', 'on(init)'],
['the-last-child', 'willRender'],
Expand All @@ -591,22 +573,18 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

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

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

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

['the-last-child', 'init'],
['the-last-child', 'didInitAttrs'],
['the-last-child', 'didReceiveAttrs'],
['the-last-child', 'on(init)']
]
Expand Down Expand Up @@ -858,21 +836,18 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

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

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

['the-bottom', 'init'],
['the-bottom', 'didInitAttrs'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)'],
['the-bottom', 'willRender'],
Expand All @@ -894,17 +869,14 @@ class LifeCycleHooksTest extends RenderingTest {
// Sync hooks

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

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

['the-bottom', 'init'],
['the-bottom', 'didInitAttrs'],
['the-bottom', 'didReceiveAttrs'],
['the-bottom', 'on(init)']
]
Expand Down Expand Up @@ -1038,7 +1010,6 @@ class LifeCycleHooksTest extends RenderingTest {
let initialHooks = (count) => {
let ret = [
['an-item', 'init'],
['an-item', 'didInitAttrs'],
['an-item', 'didReceiveAttrs'],
['an-item', 'on(init)']
];
Expand All @@ -1050,7 +1021,6 @@ class LifeCycleHooksTest extends RenderingTest {
}
ret.push(
['nested-item', 'init'],
['nested-item', 'didInitAttrs'],
['nested-item', 'didReceiveAttrs'],
['nested-item', 'on(init)']
);
Expand Down Expand Up @@ -1152,15 +1122,13 @@ class LifeCycleHooksTest extends RenderingTest {
['nested-item', 'willClearRender'],

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


['nested-item', 'init'],
['nested-item', 'didInitAttrs'],
['nested-item', 'didReceiveAttrs'],
['nested-item', 'on(init)'],
['nested-item', 'willRender'],
Expand Down Expand Up @@ -1196,12 +1164,10 @@ class LifeCycleHooksTest extends RenderingTest {

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

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

Expand Down
Loading