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 fbaf2cd
Show file tree
Hide file tree
Showing 5 changed files with 47 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 @@ -258,6 +258,24 @@ moduleFor(
this.render('<Foo/>');
this.click('button');
}

'@test Can use `on` inside conditional with modifier helper'(assert) {
assert.expect(1);

let handleClick = (value) => {
assert.equal(value, 123);
};

let Foo = defineComponent(
{ on, fn, handleClick },
'<button {{(if @handleClick (modifier "on" "click" (fn handleClick 123)))}}>Click</button>'
);

this.registerComponent('foo', { ComponentClass: Foo });

this.render('<Foo/>');
this.click('button');
}
}
);

Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { print } from '@glimmer/syntax';
import calculateLocationDisplay from '../system/calculate-location-display';
import type { EmberASTPluginEnvironment } from '../types';
import { isPath, isStringLiteral, trackLocals } from './utils';
import { BUILTIN_MODIFIERS } from '@ember/-internals/glimmer/resolver';

Check failure on line 8 in packages/ember-template-compiler/lib/plugins/transform-resolutions.ts

View workflow job for this annotation

GitHub Actions / Type Checking (current version)

'BUILTIN_MODIFIERS' is declared but its value is never read.

Check failure on line 8 in packages/ember-template-compiler/lib/plugins/transform-resolutions.ts

View workflow job for this annotation

GitHub Actions / Type Checking (current version)

Cannot find module '@ember/-internals/glimmer/resolver' or its corresponding type declarations.

Check failure on line 8 in packages/ember-template-compiler/lib/plugins/transform-resolutions.ts

View workflow job for this annotation

GitHub Actions / Debug and Prebuilt (All Tests by Package + Canary Features)

'BUILTIN_MODIFIERS' is declared but its value is never read.

Check failure on line 8 in packages/ember-template-compiler/lib/plugins/transform-resolutions.ts

View workflow job for this annotation

GitHub Actions / Debug and Prebuilt (All Tests by Package + Canary Features)

Cannot find module '@ember/-internals/glimmer/resolver' or its corresponding type declarations.

/**
@module ember
Expand Down

0 comments on commit fbaf2cd

Please sign in to comment.