From ebd45a3b242f333eb2c99c1ef7591caeb6401c78 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:48:48 -0500 Subject: [PATCH 1/3] This did not reproduce the issue..... grrr --- .../lib/suites/components.ts | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts index d4a60da81c..3c6426e1d6 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts @@ -863,6 +863,46 @@ export class GlimmerishComponents extends RenderTest { this.assertHTML('', 'destroys correctly'); } + @test({ kind: 'glimmer' }) 'destruction is not autotracked'() { + let fooCount = 0; + class Foo extends GlimmerishComponent { + declare args: { exclaim: string }; + + constructor(owner: Owner, args: Dict) { + super(owner, args); + } + + @tracked hello = 'hello'; + + get foo() { + fooCount++; + return this.hello + this.args.exclaim; + } + } + this.registerComponent('Glimmer', 'Foo', '{{this.foo}}', Foo); + + this.render('{{#if this.showing}}{{/if}}', { + showing: false, + exclaim: '?', + }); + + this.assert.strictEqual(fooCount, 0); + + this.rerender({ showing: true }); + this.assert.strictEqual(fooCount, 1); + + this.rerender({ exclaim: '!' }); + this.assert.strictEqual(fooCount, 2); + + this.rerender({ showing: false, exclaim: '!!!' }); + this.assert.strictEqual(fooCount, 2); + + this.destroy(); + this.assert.strictEqual(fooCount, 2); + + this.assertHTML('', 'destroys correctly'); + } + @test({ kind: 'templateOnly' }) 'throwing an error during rendering gives a readable error stack'(assert: Assert) { // eslint-disable-next-line no-console From 31d208f25e198825ba8f7f51683fb7ba9e7d5b01 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:52:03 -0500 Subject: [PATCH 2/3] Getting there? --- .../lib/suites/components.ts | 64 ++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts index 3c6426e1d6..2a2dcd8ded 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts @@ -864,41 +864,45 @@ export class GlimmerishComponents extends RenderTest { } @test({ kind: 'glimmer' }) 'destruction is not autotracked'() { - let fooCount = 0; - class Foo extends GlimmerishComponent { - declare args: { exclaim: string }; - - constructor(owner: Owner, args: Dict) { - super(owner, args); - } - - @tracked hello = 'hello'; - - get foo() { - fooCount++; - return this.hello + this.args.exclaim; + class State { + @tracked willDestroyCalls = 0; + increment = () => this.willDestroyCalls++; + } + let state = new State(); + class Child extends GlimmerishComponent { + declare args: { incrementWillDestroy: () => void }; + override willDestroy() { + super.willDestroy(); + this.args.incrementWillDestroy(); } } - this.registerComponent('Glimmer', 'Foo', '{{this.foo}}', Foo); - - this.render('{{#if this.showing}}{{/if}}', { - showing: false, - exclaim: '?', - }); - - this.assert.strictEqual(fooCount, 0); - - this.rerender({ showing: true }); - this.assert.strictEqual(fooCount, 1); + class Example extends GlimmerishComponent { + @tracked showChild = true; + toggleChild = () => (this.showChild = !this.showChild); + } + this.registerComponent('Glimmer', 'Child', 'a child', Child); + this.registerComponent( + 'Glimmer', + 'Example', + ` +

willDestroyCalls: {{@willDestroyCalls}}

+ - this.rerender({ exclaim: '!' }); - this.assert.strictEqual(fooCount, 2); + {{#if this.showChild}} + + {{/if}} + `, + Example + ); - this.rerender({ showing: false, exclaim: '!!!' }); - this.assert.strictEqual(fooCount, 2); + this.render( + '', + { state } + ); - this.destroy(); - this.assert.strictEqual(fooCount, 2); + this.assert.strictEqual(this.takeSnapshot(), '..'); + this.assert.strictEqual(state.willDestroyCalls, 0); + this.assertHTML('', 'p', 'destroys correctly'); this.assertHTML('', 'destroys correctly'); } From 2202359889e9c9065ec488dd1354e82d8f99fe3b Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 13 Nov 2024 16:45:21 -0500 Subject: [PATCH 3/3] Did I reproduce a different issue? --- .../lib/suites/components.ts | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts index 2a2dcd8ded..c08c2a495f 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts @@ -866,7 +866,7 @@ export class GlimmerishComponents extends RenderTest { @test({ kind: 'glimmer' }) 'destruction is not autotracked'() { class State { @tracked willDestroyCalls = 0; - increment = () => this.willDestroyCalls++; + incrementWillDestroy = () => this.willDestroyCalls++; } let state = new State(); class Child extends GlimmerishComponent { @@ -884,26 +884,62 @@ export class GlimmerishComponents extends RenderTest { this.registerComponent( 'Glimmer', 'Example', - ` -

willDestroyCalls: {{@willDestroyCalls}}

+ `

willDestroyCalls: {{@willDestroyCalls}}

{{#if this.showChild}} - {{/if}} - `, + {{/if}}`, Example ); this.render( - '', + ``, { state } ); - this.assert.strictEqual(this.takeSnapshot(), '..'); - this.assert.strictEqual(state.willDestroyCalls, 0); - this.assertHTML('', 'p', 'destroys correctly'); + // Helper because assertHTML is invisible-character sensitive, and this test doesn't care about + // that. + // Where is qunit-dom? + let output = (calls: number, hasChild: boolean) => { + if (hasChild) { + return `

willDestroyCalls: ${calls}

+ + a child +`; + } + return `

willDestroyCalls: ${calls}

+ + +`; + }; + + const el = () => this.element as unknown as HTMLElement; + const click = () => { + el().querySelector('button')?.click(); + this.rerender(); + }; + + this.assert.strictEqual(state.willDestroyCalls, 0, '0 destructions'); + this.assertHTML(output(0, true), 'initial render'); + + click(); + this.assert.strictEqual(state.willDestroyCalls, 1, '1 destruction'); + this.assertHTML(output(1, false), 'destroyed once'); + + click(); + this.assert.strictEqual(state.willDestroyCalls, 1, '1 destruction'); + this.assertHTML(output(1, true), 'shown again, no change'); + + click(); + this.assert.strictEqual(state.willDestroyCalls, 2, '2 destruction'); + this.assertHTML(output(2, false), 'destroyed twice'); + + this.destroy(); this.assertHTML('', 'destroys correctly'); }