From c0aa7179863a47dfe3842b6434dadf78a01b57f2 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 2 Apr 2019 21:20:13 -0700 Subject: [PATCH 1/2] Failing test for template-only components clearing The root cause is when the `createInstance` flag is disabled, Glimmer omits the `DidRenderLayout` opcode, which has the unintended consequence of skipping the `popBlock` call, causing an imbalance in the stack. This means additional content unrelated to the component will be treated as part of the component's bounds, causing other errors later on. It also causes other issues such as blocks not getting finalized due to the unbalanced push and pops. --- .../template-only-components-test.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/template-only-components-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/template-only-components-test.js index 9bcb62e1dd1..f35044b16d2 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/template-only-components-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/template-only-components-test.js @@ -105,6 +105,30 @@ moduleFor( this.assertInnerHTML('hello'); } + + ['@test it has the correct bounds']() { + this.registerComponent('foo-bar', 'hello'); + + this.render('outside {{#if this.isShowing}}before {{foo-bar}} after{{/if}} outside', { + isShowing: true, + }); + + this.assertInnerHTML('outside before hello after outside'); + + this.assertStableRerender(); + + runTask(() => this.context.set('isShowing', false)); + + this.assertInnerHTML('outside outside'); + + runTask(() => this.context.set('isShowing', null)); + + this.assertInnerHTML('outside outside'); + + runTask(() => this.context.set('isShowing', true)); + + this.assertInnerHTML('outside before hello after outside'); + } } ); From 9c98690108234294777cf10d6774c21944de9567 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Wed, 3 Apr 2019 09:21:04 -0700 Subject: [PATCH 2/2] [BUGFIX beta] Turn on `createInstance` for now to avoid Glimmer bug Looks like it really _needs_ to be true for input and root anyway. --- .../@ember/-internals/glimmer/lib/component-managers/input.ts | 2 +- .../@ember/-internals/glimmer/lib/component-managers/root.ts | 2 +- .../-internals/glimmer/lib/component-managers/template-only.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/input.ts b/packages/@ember/-internals/glimmer/lib/component-managers/input.ts index 1e7770362ad..22f3cf46811 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/input.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/input.ts @@ -18,7 +18,7 @@ const CAPABILITIES: ComponentCapabilities = { createCaller: true, dynamicScope: false, updateHook: true, - createInstance: false, + createInstance: true, }; export interface InputComponentState { diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/root.ts b/packages/@ember/-internals/glimmer/lib/component-managers/root.ts index a404b10a388..635a8bef97f 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/root.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/root.ts @@ -83,7 +83,7 @@ export const ROOT_CAPABILITIES: ComponentCapabilities = { createCaller: true, dynamicScope: true, updateHook: true, - createInstance: false, + createInstance: true, }; export class RootComponentDefinition implements ComponentDefinition { diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/template-only.ts b/packages/@ember/-internals/glimmer/lib/component-managers/template-only.ts index dd68327503b..2c6b3606f06 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/template-only.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/template-only.ts @@ -21,7 +21,7 @@ const CAPABILITIES: ComponentCapabilities = { createCaller: false, dynamicScope: false, updateHook: false, - createInstance: false, + createInstance: true, }; export default class TemplateOnlyComponentManager extends AbstractManager