Skip to content

Commit 9228bb8

Browse files
committed
Add tests for view cleanup/destruction.
* [Glimmer] Ensure `.parentView` is present at creation. * Add tests for `.parentView` and `.element` in each lifecycle hook. * Remove duplicated `parentView` manipulation (this is now done as part of `renderer.remove`). * Add test ensuring `willDestroyElement` is called for inverse. * Ensure element in DOM during willDestroyElement.
1 parent 186be31 commit 9228bb8

File tree

4 files changed

+159
-16
lines changed

4 files changed

+159
-16
lines changed

packages/ember-glimmer/lib/syntax/curly-component.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ class CurlyComponentManager {
7272

7373
aliasIdToElementId(args, props);
7474

75+
props.parentView = parentView;
7576
props.renderer = parentView.renderer;
7677
props[HAS_BLOCK] = hasBlock;
7778

packages/ember-glimmer/tests/integration/components/dynamic-components-test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,10 +310,15 @@ moduleFor('Components test: dynamic components', class extends RenderingTest {
310310

311311
['@test component helper destroys underlying component when it is swapped out'](assert) {
312312
let destroyed = { 'foo-bar': 0, 'foo-bar-baz': 0 };
313+
let testContext = this;
313314

314315
this.registerComponent('foo-bar', {
315316
template: 'hello from foo-bar',
316317
ComponentClass: Component.extend({
318+
willDestroyElement() {
319+
assert.equal(testContext.$(`#${this.elementId}`).length, 1, 'element is still attached to the document');
320+
},
321+
317322
willDestroy() {
318323
this._super();
319324
destroyed['foo-bar']++;

packages/ember-glimmer/tests/integration/components/life-cycle-test.js

Lines changed: 153 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,15 @@ class LifeCycleHooksTest extends RenderingTest {
88
super();
99
this.hooks = [];
1010
this.components = {};
11+
this.teardownAssertions = [];
1112
}
1213

1314
teardown() {
14-
super();
15-
this.assertHooks(
16-
'destroy',
17-
['the-top', 'willDestroyElement'],
18-
['the-middle', 'willDestroyElement'],
19-
['the-bottom', 'willDestroyElement']
20-
);
15+
super.teardown();
16+
17+
for (let i = 0; i < this.teardownAssertions.length; i++) {
18+
this.teardownAssertions[i]();
19+
}
2120
}
2221

2322
/* abstract */
@@ -51,49 +50,90 @@ class LifeCycleHooksTest extends RenderingTest {
5150
this.hooks.push(hook(name, hookName, args));
5251
};
5352

53+
let assertParentView = (hookName, instance) => {
54+
if (!instance.parentView) {
55+
this.assert.ok(false, `parentView should be present in ${hookName}`);
56+
}
57+
};
58+
59+
let assertElement = (hookName, instance) => {
60+
if (instance.tagName === '') { return; }
61+
62+
if (!instance.element) {
63+
this.assert.ok(false, `element property should be present on ${instance} during ${hookName}`);
64+
}
65+
66+
let inDOM = this.$(`#${instance.elementId}`)[0];
67+
if (!inDOM) {
68+
this.assert.ok(false, `element for ${instance} should be in the DOM during ${hookName}`);
69+
}
70+
};
71+
72+
let assertNoElement = (hookName, instance) => {
73+
if (instance.element) {
74+
this.assert.ok(false, `element should not be present in ${hookName}`);
75+
}
76+
};
77+
5478
let ComponentClass = this.ComponentClass.extend({
5579
init() {
5680
expectDeprecation(() => { this._super(...arguments); },
5781
/didInitAttrs called/);
5882

5983
pushHook('init');
6084
pushComponent(this);
85+
assertParentView('init', this);
86+
assertNoElement('init', this);
6187
},
6288

6389
didInitAttrs(options) {
6490
pushHook('didInitAttrs', options);
91+
assertParentView('didInitAttrs', this);
92+
assertNoElement('didInitAttrs', this);
6593
},
6694

6795
didUpdateAttrs(options) {
6896
pushHook('didUpdateAttrs', options);
97+
assertParentView('didUpdateAttrs', this);
6998
},
7099

71100
willUpdate(options) {
72101
pushHook('willUpdate', options);
102+
assertParentView('willUpdate', this);
73103
},
74104

75105
didReceiveAttrs(options) {
76106
pushHook('didReceiveAttrs', options);
107+
assertParentView('didReceiveAttrs', this);
77108
},
78109

79110
willRender() {
80111
pushHook('willRender');
112+
assertParentView('willRender', this);
81113
},
82114

83115
didRender() {
84116
pushHook('didRender');
117+
assertParentView('didRender', this);
118+
assertElement('didRender', this);
85119
},
86120

87121
didInsertElement() {
88122
pushHook('didInsertElement');
123+
assertParentView('didInsertElement', this);
124+
assertElement('didInsertElement', this);
89125
},
90126

91127
didUpdate(options) {
92128
pushHook('didUpdate', options);
129+
assertParentView('didUpdate', this);
130+
assertElement('didUpdate', this);
93131
},
94132

95133
willDestroyElement() {
96134
pushHook('willDestroyElement');
135+
assertParentView('willDestroyElement', this);
136+
assertElement('willDestroyElement', this);
97137
}
98138
});
99139

@@ -341,6 +381,15 @@ class LifeCycleHooksTest extends RenderingTest {
341381
['the-top', 'didRender']
342382

343383
);
384+
385+
this.teardownAssertions.push(() => {
386+
this.assertHooks(
387+
'destroy',
388+
['the-top', 'willDestroyElement'],
389+
['the-middle', 'willDestroyElement'],
390+
['the-bottom', 'willDestroyElement']
391+
);
392+
});
344393
}
345394

346395
['@test passing values through attrs causes lifecycle hooks to fire if the attribute values have changed']() {
@@ -504,8 +553,105 @@ class LifeCycleHooksTest extends RenderingTest {
504553
} else {
505554
this.assertHooks('after no-op rernder (root)');
506555
}
556+
557+
this.teardownAssertions.push(() => {
558+
this.assertHooks(
559+
'destroy',
560+
['the-top', 'willDestroyElement'],
561+
['the-middle', 'willDestroyElement'],
562+
['the-bottom', 'willDestroyElement']
563+
);
564+
});
507565
}
508566

567+
['@test components rendered from `{{each}}` have correct life-cycle hooks to be called']() {
568+
let { invoke } = this.boundHelpers;
569+
570+
this.registerComponent('an-item', { template: strip`
571+
<div>Item: {{count}}</div>
572+
` });
573+
574+
this.registerComponent('no-items', { template: strip`
575+
<div>Nothing to see here</div>
576+
` });
577+
578+
this.render(strip`
579+
{{#each items as |item|}}
580+
${invoke('an-item', { count: expr('item') })}
581+
{{else}}
582+
${invoke('no-items')}
583+
{{/each}}
584+
`, {
585+
items: [1, 2, 3, 4, 5]
586+
});
587+
588+
this.assertText('Item: 1Item: 2Item: 3Item: 4Item: 5');
589+
590+
let initialHooks = (count) => {
591+
return [
592+
['an-item', 'init'],
593+
['an-item', 'didInitAttrs', { attrs: { count } }],
594+
['an-item', 'didReceiveAttrs', { newAttrs: { count } }],
595+
['an-item', 'willRender']
596+
];
597+
};
598+
599+
let initialAfterRenderHooks = (count) => {
600+
return [
601+
['an-item', 'didInsertElement'],
602+
['an-item', 'didRender']
603+
];
604+
};
605+
606+
this.assertHooks(
607+
608+
'after initial render',
609+
610+
// Sync hooks
611+
...initialHooks(1),
612+
...initialHooks(2),
613+
...initialHooks(3),
614+
...initialHooks(4),
615+
...initialHooks(5),
616+
617+
// Async hooks
618+
...initialAfterRenderHooks(5),
619+
...initialAfterRenderHooks(4),
620+
...initialAfterRenderHooks(3),
621+
...initialAfterRenderHooks(2),
622+
...initialAfterRenderHooks(1)
623+
);
624+
625+
this.runTask(() => set(this.context, 'items', []));
626+
627+
this.assertText('Nothing to see here');
628+
629+
this.assertHooks(
630+
'reset to empty array',
631+
632+
['an-item', 'willDestroyElement'],
633+
['an-item', 'willDestroyElement'],
634+
['an-item', 'willDestroyElement'],
635+
['an-item', 'willDestroyElement'],
636+
['an-item', 'willDestroyElement'],
637+
638+
['no-items', 'init'],
639+
['no-items', 'didInitAttrs', { attrs: { } }],
640+
['no-items', 'didReceiveAttrs', { newAttrs: { } }],
641+
['no-items', 'willRender'],
642+
643+
['no-items', 'didInsertElement'],
644+
['no-items', 'didRender']
645+
);
646+
647+
this.teardownAssertions.push(() => {
648+
this.assertHooks(
649+
'destroy',
650+
651+
['no-items', 'willDestroyElement']
652+
);
653+
});
654+
}
509655
}
510656

511657
moduleFor('Components test: lifecycle hooks (curly components)', class extends LifeCycleHooksTest {

packages/ember-htmlbars/lib/morphs/morph.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,6 @@ proto.addDestruction = function(toDestroy) {
3535
};
3636

3737
proto.cleanup = function() {
38-
let view = this.emberView;
39-
40-
if (view) {
41-
let parentView = view.parentView;
42-
if (parentView && view.ownerView._destroyingSubtreeForView === parentView) {
43-
parentView.removeChild(view);
44-
}
45-
}
46-
4738
let toDestroy = this.emberToDestroy;
4839

4940
if (toDestroy) {

0 commit comments

Comments
 (0)