From df34cb4cfce9200eb119281937288ac24ee01906 Mon Sep 17 00:00:00 2001
From: patrick <pa.trickjlp@gmail.com>
Date: Thu, 21 Mar 2024 10:11:04 +0100
Subject: [PATCH] pass correct scope
---
package.json | 1 -
src/plugin.ts | 122 +++++++++++++++++++++++++++++++----------
src/scope-locals.ts | 10 +++-
test/transform.test.ts | 36 +++++++-----
4 files changed, 124 insertions(+), 45 deletions(-)
diff --git a/package.json b/package.json
index 3e9fe1f..06d1172 100644
--- a/package.json
+++ b/package.json
@@ -8,7 +8,6 @@
"scripts": {
"prepare": "tsc",
"build": "tsc",
- "pretest": "tsc",
"lint": "eslint --cache --ext .ts .",
"test": "vitest --run",
"clean": "git clean -d -f -x src __tests__"
diff --git a/src/plugin.ts b/src/plugin.ts
index 9153649..91a4dc8 100644
--- a/src/plugin.ts
+++ b/src/plugin.ts
@@ -7,7 +7,13 @@ import { JSUtils, ExtendedPluginBuilder } from './js-utils';
import type { EmberTemplateCompiler, PreprocessOptions } from './ember-template-compiler';
import { LegacyModuleName } from './public-types';
import { ScopeLocals } from './scope-locals';
-import { ASTPluginBuilder, getTemplateLocals, preprocess, print } from '@glimmer/syntax';
+import {
+ ASTPluginBuilder,
+ ASTPluginEnvironment,
+ NodeVisitor,
+ preprocess,
+ print,
+} from '@glimmer/syntax';
export * from './public-types';
@@ -372,11 +378,10 @@ function runtimeErrorIIFE(babel: typeof Babel, replacements: { ERROR_MESSAGE: st
function buildScopeLocals(
userTypedOptions: Record<string, unknown>,
- formatOptions: ModuleConfig,
- templateContent: string
+ formatOptions: ModuleConfig
): ScopeLocals {
if (formatOptions.rfc931Support && userTypedOptions.eval) {
- return discoverLocals(templateContent);
+ return new ScopeLocals();
} else if (userTypedOptions.scope) {
return userTypedOptions.scope as ScopeLocals;
} else {
@@ -384,21 +389,69 @@ function buildScopeLocals(
}
}
-function discoverLocals(templateContent: string): ScopeLocals {
- // this is wrong, but the right thing is unreleased in
- // https://github.com/glimmerjs/glimmer-vm/pull/1421, so for the moment I'm
- // sticking with the exact behavior that ember-templates-imports has.
- //
- // (the reason it's wrong is that the correct answer depends on not just the
- // template, but the ambient javascript scope. Anything in locals needs to win
- // over ember keywords. Otherwise we can never introduce new keywords.)
- let scopeLocals = new ScopeLocals();
- for (let local of getTemplateLocals(templateContent)) {
- if (local.match(/^[$A-Z_][0-9A-Z_$]*$/i)) {
- scopeLocals.add(local);
+function discoverLocals<EnvSpecificOptions>(
+ jsPath: NodePath<t.Expression>,
+ state: State<EnvSpecificOptions>,
+ locals: string[],
+ scope: ScopeLocals
+) {
+ function add(name: string) {
+ if (locals.includes(name)) return;
+ locals.push(name);
+ }
+
+ let astScope: string[] = [];
+ function isInScope(name: string) {
+ return astScope.flat().includes(name);
+ }
+
+ function isInJsScope(name: string) {
+ if (jsPath.scope.getBinding(name)) return true;
+ if (['this', 'globalThis'].includes(name)) return true;
+ if (scope.locals.includes(name)) return true;
+ if (state.originalImportedNames.has(name)) {
+ return true;
}
+ return false;
}
- return scopeLocals;
+
+ return (_env: ASTPluginEnvironment): { name: string; visitor: NodeVisitor } => {
+ return {
+ name: 'discover-locals',
+ visitor: {
+ All: {
+ enter(_node, path) {
+ const blockParams = (path.parentNode as any)?.blockParams;
+ if (blockParams && ['children', 'body'].includes(path.parentKey!)) {
+ astScope.push(blockParams);
+ }
+ },
+ exit(_node, path) {
+ const blockParams = (path.parentNode as any)?.blockParams;
+ if (blockParams && ['children', 'body'].includes(path.parentKey!)) {
+ const i = astScope.indexOf(blockParams);
+ astScope.splice(i, 1);
+ }
+ },
+ },
+ PathExpression(node) {
+ if (
+ node.head.type === 'VarHead' &&
+ !isInScope(node.head.name) &&
+ isInJsScope(node.head.name)
+ ) {
+ add(node.head.name);
+ }
+ },
+ ElementNode(node) {
+ const name = node.tag.split('.')[0];
+ if (!isInScope(name) && isInJsScope(name)) {
+ add(name);
+ }
+ },
+ },
+ };
+ };
}
function buildPrecompileOptions<EnvSpecificOptions>(
@@ -437,6 +490,9 @@ function buildPrecompileOptions<EnvSpecificOptions>(
},
};
+ const locals: string[] = [];
+ output.plugins?.ast?.push(discoverLocals(target, state, locals, scope));
+
for (let [key, value] of Object.entries(userTypedOptions)) {
if (key !== 'scope') {
// `scope` in the user-facing API becomes `locals` in the low-level
@@ -445,7 +501,7 @@ function buildPrecompileOptions<EnvSpecificOptions>(
}
}
- output.locals = scope.locals;
+ output.locals = locals;
if (config.rfc931Support) {
output.strictMode = true;
@@ -483,7 +539,7 @@ function insertCompiledTemplate<EnvSpecificOptions>(
backingClass: NodePath<Parameters<typeof t.callExpression>[1][number]> | undefined
) {
let t = babel.types;
- let scopeLocals = buildScopeLocals(userTypedOptions, config, template);
+ let scopeLocals = buildScopeLocals(userTypedOptions, config);
let options = buildPrecompileOptions(
babel,
target,
@@ -513,6 +569,8 @@ function insertCompiledTemplate<EnvSpecificOptions>(
configFile: false,
}) as t.File;
+ scopeLocals.locals.length = 0;
+ scopeLocals.locals.push(...options.locals!);
ensureImportedNames(target, scopeLocals, state.util, state.originalImportedNames);
remapIdentifiers(precompileResultAST, babel, scopeLocals);
@@ -560,7 +618,7 @@ function insertTransformedTemplate<EnvSpecificOptions>(
backingClass: NodePath<Parameters<typeof t.callExpression>[1][number]> | undefined
) {
let t = babel.types;
- let scopeLocals = buildScopeLocals(userTypedOptions, formatOptions, template);
+ let scopeLocals = buildScopeLocals(userTypedOptions, formatOptions);
let options = buildPrecompileOptions(
babel,
target,
@@ -574,14 +632,14 @@ function insertTransformedTemplate<EnvSpecificOptions>(
let transformed = print(ast, { entityEncoding: 'raw' });
if (target.isCallExpression()) {
(target.get('arguments.0') as NodePath<t.Node>).replaceWith(t.stringLiteral(transformed));
- if (!scopeLocals.isEmpty()) {
- if (!formatOptions.enableScope) {
- maybePruneImport(state.util, target.get('callee'));
- target.set('callee', precompileTemplate(state.util, target));
- }
- ensureImportedNames(target, scopeLocals, state.util, state.originalImportedNames);
- updateScope(babel, target, scopeLocals);
+ if (!formatOptions.enableScope) {
+ maybePruneImport(state.util, target.get('callee'));
+ target.set('callee', precompileTemplate(state.util, target));
}
+ scopeLocals.locals.length = 0;
+ scopeLocals.locals.push(...options.locals!);
+ ensureImportedNames(target, scopeLocals, state.util, state.originalImportedNames);
+ updateScope(babel, target, scopeLocals);
if (formatOptions.rfc931Support === 'polyfilled') {
maybePruneImport(state.util, target.get('callee'));
@@ -615,6 +673,8 @@ function insertTransformedTemplate<EnvSpecificOptions>(
t.callExpression(precompileTemplate(state.util, target), [t.stringLiteral(transformed)])
)[0];
ensureImportedNames(newCall, scopeLocals, state.util, state.originalImportedNames);
+ scopeLocals.locals.length = 0;
+ scopeLocals.locals.push(...options.locals!);
updateScope(babel, newCall, scopeLocals);
} else {
(target.get('quasi').get('quasis.0') as NodePath<t.TemplateElement>).replaceWith(
@@ -635,6 +695,7 @@ function templateFactoryConfig(opts: NormalizedOpts) {
function buildScope(babel: typeof Babel, locals: ScopeLocals) {
let t = babel.types;
+
return t.arrowFunctionExpression(
[],
t.objectExpression(
@@ -656,13 +717,16 @@ function updateScope(babel: typeof Babel, target: NodePath<t.CallExpression>, lo
});
if (scope) {
scope.set('value', buildScope(babel, locals));
- } else {
+ if (locals.isEmpty()) {
+ scope.remove();
+ }
+ } else if (!locals.isEmpty()) {
secondArg.pushContainer(
'properties',
t.objectProperty(t.identifier('scope'), buildScope(babel, locals))
);
}
- } else {
+ } else if (!locals.isEmpty()) {
target.pushContainer(
'arguments',
t.objectExpression([t.objectProperty(t.identifier('scope'), buildScope(babel, locals))])
diff --git a/src/scope-locals.ts b/src/scope-locals.ts
index b283dc9..e002d02 100644
--- a/src/scope-locals.ts
+++ b/src/scope-locals.ts
@@ -31,7 +31,15 @@ export class ScopeLocals {
}
entries() {
- return Object.entries(this.#mapping);
+ let mapping: Record<string, string> = {};
+ this.locals.forEach((name) => {
+ mapping[name] = this.mapping[name] || name;
+ });
+ return Object.entries(mapping);
+ }
+
+ get mapping() {
+ return this.#mapping;
}
add(key: string, value?: string) {
diff --git a/test/transform.test.ts b/test/transform.test.ts
index ee9c16e..5e74982 100644
--- a/test/transform.test.ts
+++ b/test/transform.test.ts
@@ -12,7 +12,7 @@ import sinon from 'sinon';
import { ExtendedPluginBuilder } from '../src/js-utils';
import * as precompile from './mock-precompile';
import { Preprocessor } from 'content-tag';
-import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
describe('htmlbars-inline-precompile', function () {
// eslint-disable-next-line @typescript-eslint/no-var-requires
@@ -1115,6 +1115,7 @@ describe('htmlbars-inline-precompile', function () {
const template = precompileTemplate("<Message @text={{two}} />", {
moduleName: 'customModuleName',
scope: () => ({
+ Message,
two
})
});"
@@ -1537,7 +1538,7 @@ describe('htmlbars-inline-precompile', function () {
describe('scope', function () {
it('correctly handles scope function (non-block arrow function)', function () {
- let source = 'hello';
+ let source = '<foo /><bar/>';
let spy = sinon.spy(compiler, 'precompile');
transform(
@@ -1547,7 +1548,7 @@ describe('htmlbars-inline-precompile', function () {
});
it('correctly handles scope function (block arrow function)', function () {
- let source = 'hello';
+ let source = '<foo /><bar/>';
let spy = sinon.spy(compiler, 'precompile');
transform(
@@ -1558,7 +1559,7 @@ describe('htmlbars-inline-precompile', function () {
});
it('correctly handles scope function (normal function)', function () {
- let source = 'hello';
+ let source = '<foo /><bar/>';
let spy = sinon.spy(compiler, 'precompile');
transform(
@@ -1569,7 +1570,7 @@ describe('htmlbars-inline-precompile', function () {
});
it('correctly handles scope function (object method)', function () {
- let source = 'hello';
+ let source = '<foo /><bar/>';
let spy = sinon.spy(compiler, 'precompile');
transform(
@@ -1579,7 +1580,7 @@ describe('htmlbars-inline-precompile', function () {
});
it('correctly handles scope function with coverage', function () {
- let source = 'hello';
+ let source = '<foo /><bar/>';
let spy = sinon.spy(compiler, 'precompile');
transform(
@@ -1674,7 +1675,7 @@ describe('htmlbars-inline-precompile', function () {
// whether you've (for example) capitalized your variable identifier.
//
// needs https://github.com/glimmerjs/glimmer-vm/pull/1421
- it.skip('shadows html elements with locals', function () {
+ it('shadows html elements with locals', function () {
plugins = [
[
HTMLBarsInlinePrecompile,
@@ -1693,11 +1694,16 @@ describe('htmlbars-inline-precompile', function () {
);
expect(transformed).toMatchInlineSnapshot(`
- import templateOnly from "@ember/component/template-only";
+ "import { precompileTemplate } from "@ember/template-compilation";
import { setComponentTemplate } from "@ember/component";
- import { precompileTemplate } from "@ember/template-compilation";
+ import templateOnly from "@ember/component/template-only";
let div = 1;
- export default setComponentTemplate(precompileTemplate('<div></div>', { scope: () => ({ div }), strictMode: true }), templateOnly());
+ export default setComponentTemplate(precompileTemplate("<div></div>", {
+ scope: () => ({
+ div
+ }),
+ strictMode: true
+ }), templateOnly());"
`);
});
@@ -1734,7 +1740,7 @@ describe('htmlbars-inline-precompile', function () {
});
// needs https://github.com/glimmerjs/glimmer-vm/pull/1421
- it.skip('leaves ember keywords alone when no local is defined', function () {
+ it('leaves ember keywords alone when no local is defined', function () {
plugins = [
[
HTMLBarsInlinePrecompile,
@@ -1752,10 +1758,12 @@ describe('htmlbars-inline-precompile', function () {
);
expect(transformed).toMatchInlineSnapshot(`
- import templateOnly from "@ember/component/template-only";
+ "import { precompileTemplate } from "@ember/template-compilation";
import { setComponentTemplate } from "@ember/component";
- import { precompileTemplate } from "@ember/template-compilation";
- export default setComponentTemplate(precompileTemplate('{{hasBlock "thing"}}', { strictMode: true }), templateOnly());
+ import templateOnly from "@ember/component/template-only";
+ export default setComponentTemplate(precompileTemplate("{{hasBlock \\"thing\\"}}", {
+ strictMode: true
+ }), templateOnly());"
`);
});