Skip to content

Commit b92730a

Browse files
committed
[bugfix] allow modifier to resolve built-ins like on
partially fixes emberjs#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
1 parent be51c93 commit b92730a

File tree

4 files changed

+29
-1
lines changed

4 files changed

+29
-1
lines changed

packages/@ember/-internals/glimmer/lib/resolver.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ const BUILTIN_KEYWORD_MODIFIERS: Record<string, ModifierDefinitionState> = {
151151
action: actionModifier,
152152
};
153153

154-
const BUILTIN_MODIFIERS: Record<string, object> = {
154+
export const BUILTIN_MODIFIERS: Record<string, object> = {
155155
...BUILTIN_KEYWORD_MODIFIERS,
156156
on,
157157
};

packages/@ember/-internals/glimmer/lib/setup-registry.ts

+5
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Renderer } from './renderer';
1212
import OutletTemplate from './templates/outlet';
1313
import RootTemplate from './templates/root';
1414
import OutletView from './views/outlet';
15+
import { BUILTIN_MODIFIERS } from './resolver';
1516

1617
export function setupApplicationRegistry(registry: Registry): void {
1718
// because we are using injections we can't use instantiate false
@@ -57,4 +58,8 @@ export function setupEngineRegistry(registry: Registry): void {
5758
if (!ENV._TEMPLATE_ONLY_GLIMMER_COMPONENTS) {
5859
registry.register(P`component:-default`, Component);
5960
}
61+
62+
for (let [name, modifier] of Object.entries(BUILTIN_MODIFIERS)) {
63+
registry.register(`modifier:${name}`, modifier, { instantiate: false });
64+
}
6065
}

packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js

+22
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,28 @@ moduleFor(
476476
this.assertStableRerender();
477477
}
478478

479+
'@test can resolve built-in modifiers'(assert) {
480+
let wasCalled = false;
481+
let id = 'wow-what-an-original-id';
482+
this.render(
483+
`<div id='${id}' {{(if this.isModifying (modifier "on" "click" this.callAction))}} />`,
484+
{
485+
callAction: () => {
486+
wasCalled = true;
487+
},
488+
}
489+
);
490+
491+
let element = document.querySelector(`#${id}`);
492+
element.click();
493+
494+
assert.false(wasCalled, 'modifier should not be set up');
495+
496+
runTask(() => set(this.context, 'isModifying', true));
497+
element.click();
498+
assert.true(wasCalled, 'on modifier can be used');
499+
}
500+
479501
'@test Cannot dynamically resolve a modifier'(assert) {
480502
this.registerModifier(
481503
'replace',

packages/ember-template-compiler/lib/plugins/transform-resolutions.ts

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { print } from '@glimmer/syntax';
55
import calculateLocationDisplay from '../system/calculate-location-display';
66
import type { EmberASTPluginEnvironment } from '../types';
77
import { isPath, isStringLiteral, trackLocals } from './utils';
8+
import { BUILTIN_MODIFIERS } from '@ember/-internals/glimmer/resolver';
89

910
/**
1011
@module ember

0 commit comments

Comments
 (0)