Skip to content

Commit

Permalink
Merge pull request #15513 from mitchlloyd/mitchlloyd/destroying-appen…
Browse files Browse the repository at this point in the history
…ded-components

Release root components after they are destroyed
  • Loading branch information
rwjblue authored Aug 7, 2017
2 parents 12f4170 + bd60d85 commit 81baec7
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 10 deletions.
1 change: 1 addition & 0 deletions packages/ember-glimmer/lib/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ class Renderer {
let root = roots[i];
if (root.isFor(view)) {
root.destroy();
roots.splice(i, 1);
}
}
}
Expand Down
30 changes: 20 additions & 10 deletions packages/ember-glimmer/tests/integration/components/append-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,16 +317,29 @@ class AbstractAppendTest extends RenderingTest {

this.runTask(() => this.component.destroy());

if (this.isHTMLBars) {
// Bug in Glimmer – component should not have .element at this point
assert.ok(!this.component.element, 'It should not have an element');
}

assert.ok(!this.component.element, 'It should not have an element');
assert.ok(!componentElement.parentElement, 'The component element should be detached');

this.assert.equal(willDestroyCalled, 1);
}

['@test releasing a root component after it has been destroy'](assert) {
let renderer = this.owner.lookup('renderer:-dom');

this.registerComponent('x-component', {
ComponentClass: Component.extend()
});

this.component = this.owner.factoryFor('component:x-component').create();
this.append(this.component);

assert.equal(renderer._roots.length, 1, 'added a root component');

this.runTask(() => this.component.destroy());

assert.equal(renderer._roots.length, 0, 'released the root component');
}

['@test appending, updating and destroying multiple components'](assert) {
let willDestroyCalled = 0;

Expand Down Expand Up @@ -411,11 +424,8 @@ class AbstractAppendTest extends RenderingTest {
second.destroy();
});

if (this.isHTMLBars) {
// Bug in Glimmer – component should not have .element at this point
assert.ok(!first.element, 'The first component should not have an element');
assert.ok(!second.element, 'The second component should not have an element');
}
assert.ok(!first.element, 'The first component should not have an element');
assert.ok(!second.element, 'The second component should not have an element');

assert.ok(!componentElement1.parentElement, 'The first component element should be detached');
assert.ok(!componentElement2.parentElement, 'The second component element should be detached');
Expand Down

0 comments on commit 81baec7

Please sign in to comment.