diff --git a/packages/@glimmer-workspace/integration-tests/test/modifiers/dynamic-modifiers-test.ts b/packages/@glimmer-workspace/integration-tests/test/modifiers/dynamic-modifiers-test.ts index 91854bdbf7e..6607b1204c2 100644 --- a/packages/@glimmer-workspace/integration-tests/test/modifiers/dynamic-modifiers-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/modifiers/dynamic-modifiers-test.ts @@ -87,6 +87,52 @@ class DynamicModifiersResolutionModeTest extends RenderTest { assert.deepEqual(receivedArgs, ['y', 'x']); } + @test + 'Dynamic modifier becoming undefined does not error when args change afterwards'(assert: Assert) { + let installCount = 0; + let lastValue: unknown; + + // The modifier reads args[0], so its tag tracks the arg's reactivity. + // When outer changes and the modifier's tag is invalidated, the bug triggers: + // scheduleUpdateModifier would be called with undefined instance. + const foo = defineSimpleModifier((element: Element, args: unknown[]) => { + installCount++; + lastValue = args[0]; + (element as HTMLElement).dataset['value'] = String(args[0]); + }); + + this.render( + ` + {{~#let (modifier this.foo this.outer) as |bar|~}} +
content
+ {{~/let~}} + `, + { foo, outer: 'initial', cond: true } + ); + + this.assertHTML('
content
'); + assert.strictEqual(installCount, 1, 'modifier installed once on initial render'); + assert.strictEqual(lastValue, 'initial'); + + // Disable the modifier — newInstance becomes undefined, instance is destroyed + // (data-value persists because no cleanup function was returned) + this.rerender({ cond: false }); + this.assertHTML('
content
'); + + // Change args while the modifier is disabled — this triggered the bug: + // "Cannot destructure property 'manager' of '.for' as it is undefined" + // because the modifier's tag was invalidated by the arg change, and the + // stale tag caused scheduleUpdateModifier to be called with undefined. + this.rerender({ outer: 'changed' }); + this.assertHTML('
content
'); + + // Re-enable the modifier — should install cleanly with updated args + this.rerender({ cond: true }); + this.assertHTML('
content
'); + assert.strictEqual(installCount, 2, 'modifier installed again after re-enabling'); + assert.strictEqual(lastValue, 'changed'); + } + @test 'Can pass curried modifier as argument and invoke dynamically (with args)'() { const foo = defineSimpleModifier( diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts index e12d7f8fbfb..b6687aa8183 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts @@ -363,11 +363,16 @@ export class UpdateDynamicModifierOpcode implements UpdatingOpcode { this.tag = tag; vm.env.scheduleInstallModifier(newInstance); + } else { + tag = null; + this.tag = null; } this.instance = newInstance; } else if (tag !== null && !validateTag(tag, lastUpdated)) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- @fixme + // tag !== null guarantees instance is defined: this.tag is only set to a + // non-null value alongside this.instance being set to a non-null value. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion vm.env.scheduleUpdateModifier(instance!); this.lastUpdated = valueForTag(tag); }