Skip to content

Commit

Permalink
[bugfix] allow modifier to resolve built-ins like on
Browse files Browse the repository at this point in the history
partially fixes #19869

Due to [GlimmerVM's assertion][1], this still does not fix the issue in
strict mode. We also can't work around the strict mode limitation with
an AST transform since Glimmer's transforms run first.

[1]: https://github.com/glimmerjs/glimmer-vm/blob/2ddbbc4a9b97db4f326c4d92021f089c464ab093/packages/%40glimmer/compiler/lib/passes/1-normalization/keywords/utils/curry.ts#L53
  • Loading branch information
fivetanley committed Jan 24, 2024
1 parent be51c93 commit 834da17
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/@ember/-internals/glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const BUILTIN_KEYWORD_MODIFIERS: Record<string, ModifierDefinitionState> = {
action: actionModifier,
};

const BUILTIN_MODIFIERS: Record<string, object> = {
export const BUILTIN_MODIFIERS: Record<string, object> = {
...BUILTIN_KEYWORD_MODIFIERS,
on,
};
Expand Down
5 changes: 5 additions & 0 deletions packages/@ember/-internals/glimmer/lib/setup-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Renderer } from './renderer';
import OutletTemplate from './templates/outlet';
import RootTemplate from './templates/root';
import OutletView from './views/outlet';
import { BUILTIN_MODIFIERS } from './resolver';

export function setupApplicationRegistry(registry: Registry): void {
// because we are using injections we can't use instantiate false
Expand Down Expand Up @@ -57,4 +58,8 @@ export function setupEngineRegistry(registry: Registry): void {
if (!ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {
registry.register(P`component:-default`, Component);
}

for (let [name, modifier] of Object.entries(BUILTIN_MODIFIERS)) {
registry.register(`modifier:${name}`, modifier, { instantiate: false });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,28 @@ moduleFor(
this.assertStableRerender();
}

'@test can resolve built-in modifiers'(assert) {
let wasCalled = false;
let id = 'wow-what-an-original-id';
this.render(
`<div id='${id}' {{(if this.isModifying (modifier "on" "click" this.callAction))}} />`,
{
callAction: () => {
wasCalled = true;
},
}
);

let element = document.querySelector(`#${id}`);
element.click();

assert.false(wasCalled, 'modifier should not be set up');

runTask(() => set(this.context, 'isModifying', true));
element.click();
assert.true(wasCalled, 'on modifier can be used');
}

'@test Cannot dynamically resolve a modifier'(assert) {
this.registerModifier(
'replace',
Expand Down

0 comments on commit 834da17

Please sign in to comment.