Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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|~}}
<div {{ (if this.cond bar) }}>content</div>
{{~/let~}}
`,
{ foo, outer: 'initial', cond: true }
);

this.assertHTML('<div data-value="initial">content</div>');
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('<div data-value="initial">content</div>');

// 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('<div data-value="initial">content</div>');

// Re-enable the modifier — should install cleanly with updated args
this.rerender({ cond: true });
this.assertHTML('<div data-value="changed">content</div>');
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(
Expand Down
7 changes: 6 additions & 1 deletion packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading