Skip to content

Commit 013205f

Browse files
NullVoxPopuli-ai-agentNullVoxPopuliclaude
authored
Remove globalThis for keyword resolution in runtime template() (#21254)
* Remove globalThis usage for keyword resolution in runtime template() Instead of storing keywords on globalThis (which collides when multiple ember-source instances exist on a page), provide keyword values through the template evaluator: - For the eval form: wrap the compiled source in a function that receives keyword values as parameters while preserving access to the caller's lexical scope via direct eval - For the scope form: merge keywords into the resolved scope (already worked, just needed the globalThis removal) - For neither: inject keywords via new Function parameters The auto-import-builtins AST plugin no longer rewrites keyword references at runtime since lexicalScope now returns true for keywords, causing hasLocal() to return true and skip the rewrite. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Use WeakMap for compile state, fix lint issues Replace module-level _compileState mutable with a WeakMap keyed on the options object. This prevents state leaking between template() calls and ensures entries are GC'd when the options object is no longer referenced. Also removes unused EmberPrecompileOptions import and fixes prettier formatting. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Simplify: use auto-import-builtins rewrite with __keywords__ container Instead of making lexicalScope return true for each keyword and tracking which ones need injection, let auto-import-builtins rewrite keyword references to __keywords__.on (pointing at our keywords object) and inject a single __keywords__ container variable into the evaluation scope. This eliminates the WeakMap side channel entirely and restores the original buildEvaluator-before-compilation ordering. The only evaluator change is injecting one extra scope variable (__keywords__) that holds the keywords object. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Rename __keywords__ to __ember_keywords__ Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Rename KEYWORDS_VAR to RUNTIME_KEYWORDS_NAME Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Fix prettier formatting Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: NullVoxPopuli <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
1 parent a5c1907 commit 013205f

3 files changed

Lines changed: 46 additions & 20 deletions

File tree

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@
293293
"@ember/template-compiler/lib/plugins/assert-against-named-outlets.js": "ember-source/@ember/template-compiler/lib/plugins/assert-against-named-outlets.js",
294294
"@ember/template-compiler/lib/plugins/assert-input-helper-without-block.js": "ember-source/@ember/template-compiler/lib/plugins/assert-input-helper-without-block.js",
295295
"@ember/template-compiler/lib/plugins/assert-reserved-named-arguments.js": "ember-source/@ember/template-compiler/lib/plugins/assert-reserved-named-arguments.js",
296+
"@ember/template-compiler/lib/plugins/auto-import-builtins.js": "ember-source/@ember/template-compiler/lib/plugins/auto-import-builtins.js",
296297
"@ember/template-compiler/lib/plugins/index.js": "ember-source/@ember/template-compiler/lib/plugins/index.js",
297298
"@ember/template-compiler/lib/plugins/transform-action-syntax.js": "ember-source/@ember/template-compiler/lib/plugins/transform-action-syntax.js",
298299
"@ember/template-compiler/lib/plugins/transform-each-in-into-each.js": "ember-source/@ember/template-compiler/lib/plugins/transform-each-in-into-each.js",
@@ -384,4 +385,4 @@
384385
}
385386
},
386387
"packageManager": "[email protected]"
387-
}
388+
}

packages/@ember/template-compiler/lib/compile-options.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,18 @@ function malformedComponentLookup(string: string) {
1515
return string.indexOf('::') === -1 && string.indexOf(':') > -1;
1616
}
1717

18-
const RUNTIME_KEYWORDS_NAME = '__ember_keywords__';
19-
export const keywords = {
18+
/**
19+
* The variable name used to inject the keywords object into the
20+
* template's evaluation scope. auto-import-builtins rewrites bare
21+
* keyword references (e.g. `on`) to property accesses on this
22+
* variable (e.g. `__ember_keywords__.on`).
23+
*/
24+
export const RUNTIME_KEYWORDS_NAME = '__ember_keywords__';
25+
26+
export const keywords: Record<string, unknown> = {
2027
on,
2128
};
2229

23-
// Not worth adding a type
24-
(globalThis as any)[RUNTIME_KEYWORDS_NAME] = keywords;
25-
2630
function buildCompileOptions(_options: EmberPrecompileOptions): EmberPrecompileOptions {
2731
let moduleName = _options.moduleName;
2832

@@ -45,18 +49,13 @@ function buildCompileOptions(_options: EmberPrecompileOptions): EmberPrecompileO
4549

4650
options.meta ||= {};
4751
options.meta.emberRuntime ||= {
48-
/**
49-
* NOTE: when stepping through lexicalScope, or other callbacks here,
50-
* we first detect the keywords as "not in scope",
51-
* and that is what we want, so that we can import them.
52-
*/
5352
lookupKeyword(name: string): string {
5453
assert(
5554
`${name} is not a known keyword. Available keywords: ${Object.keys(keywords).join(', ')}`,
5655
name in keywords
5756
);
5857

59-
return `globalThis.${RUNTIME_KEYWORDS_NAME}.${name}`;
58+
return `${RUNTIME_KEYWORDS_NAME}.${name}`;
6059
},
6160
};
6261

@@ -65,6 +64,12 @@ function buildCompileOptions(_options: EmberPrecompileOptions): EmberPrecompileO
6564
const globalScopeEvaluator = (value: string) => new Function(`return ${value};`)();
6665

6766
options.lexicalScope = (variable: string) => {
67+
// The keywords container variable is always "in scope" —
68+
// we inject it via the evaluator in template.ts.
69+
if (variable === RUNTIME_KEYWORDS_NAME) {
70+
return true;
71+
}
72+
6873
if (ALLOWED_GLOBALS.has(variable)) {
6974
return variable in globalThis;
7075
}
@@ -82,11 +87,18 @@ function buildCompileOptions(_options: EmberPrecompileOptions): EmberPrecompileO
8287
if ('scope' in options) {
8388
const scope = (options.scope as () => Record<string, unknown>)();
8489

85-
options.lexicalScope = (variable: string) => variable in scope || variable in keywords;
90+
options.lexicalScope = (variable: string) =>
91+
variable in scope || variable === RUNTIME_KEYWORDS_NAME;
8692

8793
delete options.scope;
8894
}
8995

96+
// When neither eval nor scope is provided, the keywords container
97+
// still needs to be visible to the compiler.
98+
if (!options.lexicalScope && Object.keys(keywords).length > 0) {
99+
options.lexicalScope = (variable: string) => variable === RUNTIME_KEYWORDS_NAME;
100+
}
101+
90102
if ('locals' in options && !options.locals) {
91103
// Glimmer's precompile options declare `locals` like:
92104
// locals?: string[]

packages/@ember/template-compiler/lib/template.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { precompile as glimmerPrecompile } from '@glimmer/compiler';
33
import type { SerializedTemplateWithLazyBlock } from '@glimmer/interfaces';
44
import { setComponentTemplate } from '@glimmer/manager';
55
import { templateFactory } from '@glimmer/opcode-compiler';
6-
import compileOptions, { keywords } from './compile-options';
6+
import compileOptions, { keywords, RUNTIME_KEYWORDS_NAME } from './compile-options';
77
import type { EmberPrecompileOptions } from './types';
88

99
type ComponentClass = abstract new (...args: any[]) => object;
@@ -265,19 +265,32 @@ const evaluator = (source: string) => {
265265
*/
266266
function buildEvaluator(options: Partial<EmberPrecompileOptions>) {
267267
if (options.eval) {
268-
return options.eval;
268+
const userEval = options.eval;
269+
270+
// Wrap the compiled source in a function that receives the keywords
271+
// container as a parameter. The user's eval evaluates this in the
272+
// caller's scope, so local variables (like `handleClick`) are captured
273+
// via closure, while `__keywords__` comes from the function parameter.
274+
return (source: string) => {
275+
let wrapperFn = userEval(`(function(${RUNTIME_KEYWORDS_NAME}){ return (${source}); })`) as (
276+
...args: unknown[]
277+
) => unknown;
278+
279+
return wrapperFn(keywords);
280+
};
269281
} else {
270-
/**
271-
* This is ran before the template is compiled,
272-
* so we cannot use any information gathered during template compilation.
273-
*/
274282
let scope = options.scope?.();
275283

276284
if (!scope) {
285+
if (Object.keys(keywords).length > 0) {
286+
return (source: string) => {
287+
return new Function(RUNTIME_KEYWORDS_NAME, `return (${source})`)(keywords);
288+
};
289+
}
277290
return evaluator;
278291
}
279292

280-
scope = Object.assign({}, keywords, scope);
293+
scope = Object.assign({ [RUNTIME_KEYWORDS_NAME]: keywords }, scope);
281294

282295
return (source: string) => {
283296
let hasThis = Object.prototype.hasOwnProperty.call(scope, 'this');

0 commit comments

Comments
 (0)