From 099d112bac2c4f551e240d309cbc8e23e04c92c6 Mon Sep 17 00:00:00 2001 From: Mitch Lloyd Date: Sun, 16 Jul 2017 15:38:35 -0700 Subject: [PATCH 1/2] Run skipped Glimmer assertions --- .../tests/integration/components/append-test.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/ember-glimmer/tests/integration/components/append-test.js b/packages/ember-glimmer/tests/integration/components/append-test.js index d973f495480..9d574e31ddc 100644 --- a/packages/ember-glimmer/tests/integration/components/append-test.js +++ b/packages/ember-glimmer/tests/integration/components/append-test.js @@ -317,11 +317,7 @@ 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); @@ -411,11 +407,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'); From bd60d853d76252e379dc4876b24475cb3c5d3a10 Mon Sep 17 00:00:00 2001 From: Mitch Lloyd Date: Sun, 16 Jul 2017 15:39:42 -0700 Subject: [PATCH 2/2] Release reference to root components after destroy --- packages/ember-glimmer/lib/renderer.js | 1 + .../tests/integration/components/append-test.js | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/packages/ember-glimmer/lib/renderer.js b/packages/ember-glimmer/lib/renderer.js index e598f0d96e8..29cf5c9b7cf 100644 --- a/packages/ember-glimmer/lib/renderer.js +++ b/packages/ember-glimmer/lib/renderer.js @@ -241,6 +241,7 @@ class Renderer { let root = roots[i]; if (root.isFor(view)) { root.destroy(); + roots.splice(i, 1); } } } diff --git a/packages/ember-glimmer/tests/integration/components/append-test.js b/packages/ember-glimmer/tests/integration/components/append-test.js index 9d574e31ddc..75eb9691867 100644 --- a/packages/ember-glimmer/tests/integration/components/append-test.js +++ b/packages/ember-glimmer/tests/integration/components/append-test.js @@ -323,6 +323,23 @@ class AbstractAppendTest extends RenderingTest { 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;