From 07b470311c08a1c73db0887647bba9f8070d7dc6 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 14 Jan 2025 11:06:39 -0500 Subject: [PATCH 1/8] Prototype failing test --- __tests__/tests.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index 22c9c20..e90bcce 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -1901,6 +1901,34 @@ describe('htmlbars-inline-precompile', function () { `); }); + it('uses allowed platform globals when used', function () { + plugins = [ + [ + HTMLBarsInlinePrecompile, + { + compiler, + targetFormat: 'hbs', + }, + ], + ]; + + let transformed = transform( + `import { template } from '@ember/template-compiler'; + const data = {}; + export default template('{{JSON.stringify data}}', { eval: function() { return eval(arguments[0]) } }) + ` + ); + + expect(transformed).toEqualCode(` + import HelloWorld from "somewhere"; + import { precompileTemplate } from "@ember/template-compilation"; + import { setComponentTemplate } from "@ember/component"; + import templateOnly from "@ember/component/template-only"; + const data = {}; + export default setComponentTemplate(precompileTemplate('{{JSON.stringify data}}', { strictMode: true, scope: () => ({ JSON, data }) }), templateOnly()); + `); + }); + // You might think this would be confusing style, and you'd be correct. But // that's what the lint rules are for. When it comes to correctness, we need // our scope to behave like real Javascript, and Javascript doesn't care From 82ee5508f77129a361ce7cd3fd1eccb6b33acbb4 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 14 Jan 2025 13:48:08 -0500 Subject: [PATCH 2/8] yay --- __tests__/tests.ts | 1 - src/scope-locals.ts | 41 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index e90bcce..5e78ee8 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -1920,7 +1920,6 @@ describe('htmlbars-inline-precompile', function () { ); expect(transformed).toEqualCode(` - import HelloWorld from "somewhere"; import { precompileTemplate } from "@ember/template-compilation"; import { setComponentTemplate } from "@ember/component"; import templateOnly from "@ember/component/template-only"; diff --git a/src/scope-locals.ts b/src/scope-locals.ts index c4ca85d..979581b 100644 --- a/src/scope-locals.ts +++ b/src/scope-locals.ts @@ -12,6 +12,45 @@ import { ASTPluginEnvironment, NodeVisitor } from '@glimmer/syntax'; import { astNodeHasBinding } from './hbs-utils'; import { readOnlyArray } from './read-only-array'; +/** + * RFC: Pending.... + * + * tl;dr: + * - utilities that begin with uppercase letter + * OR are guaranteed to never be added to glimmer as a keyword (e.g.: globalThis) + * - must not need `new` to invoke + * + * Ref: + * - https://tc39.es/ecma262/#sec-global-object + * - https://tc39.es/ecma262/#sec-function-properties-of-the-global-object + */ +const ALLOWED_GLOBALS = new Set([ + // namespaces + 'globalThis', + 'JSON', + 'Math', + 'Atomics', + 'Reflect', + // functions + 'isNaN', + 'isFinite', + 'parseInt', + 'parseFloat', + 'decodeURI', + 'decodeURIComponent', + 'encodeURI', + 'encodeURIComponent', + // new-less Constructors (still functions) + 'Number', + 'Object', // different behavior from (hash) + 'Array', // different behavior from (array) + 'String', + 'BigInt', + 'Date', + // Values + 'Infinity', +]); + /* `mode` refers to the implicit and explicit formats defined here: @@ -72,7 +111,7 @@ export class ScopeLocals { #isInJsScope(hbsName: string, jsPath: NodePath) { let jsName = this.#mapping[hbsName] ?? hbsName; - return ['globalThis'].includes(jsName) || jsPath.scope.getBinding(jsName); + return ALLOWED_GLOBALS.has(jsName) || jsPath.scope.getBinding(jsName); } // this AST transform discovers all possible upvars in HBS that refer to valid From e364ea9bd1c3bd445e180fb274a7d59466c56088 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Sat, 1 Mar 2025 16:05:04 -0500 Subject: [PATCH 3/8] implement RFC 1070 --- __tests__/tests.ts | 39 +++++++++++++++++------------- src/scope-locals.ts | 59 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 67 insertions(+), 31 deletions(-) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index 5e78ee8..d5314f7 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -12,6 +12,7 @@ import sinon from 'sinon'; import { ExtendedPluginBuilder } from '../src/js-utils'; import 'code-equality-assertions/jest'; import { Preprocessor } from 'content-tag'; +import { ALLOWED_GLOBALS } from '../src/scope-locals'; describe('htmlbars-inline-precompile', function () { // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -1901,31 +1902,35 @@ describe('htmlbars-inline-precompile', function () { `); }); - it('uses allowed platform globals when used', function () { - plugins = [ - [ - HTMLBarsInlinePrecompile, - { - compiler, - targetFormat: 'hbs', - }, - ], - ]; - - let transformed = transform( - `import { template } from '@ember/template-compiler'; + describe('implements RFC#1070: default globals', function () { + for (let name of ALLOWED_GLOBALS) { + it(`${name}: allowed`, function () { + plugins = [ + [ + HTMLBarsInlinePrecompile, + { + compiler, + targetFormat: 'hbs', + }, + ], + ]; + + let transformed = transform( + `import { template } from '@ember/template-compiler'; const data = {}; - export default template('{{JSON.stringify data}}', { eval: function() { return eval(arguments[0]) } }) + export default template('{{${name} data}}', { eval: function() { return eval(arguments[0]) } }) ` - ); + ); - expect(transformed).toEqualCode(` + expect(transformed).toEqualCode(` import { precompileTemplate } from "@ember/template-compilation"; import { setComponentTemplate } from "@ember/component"; import templateOnly from "@ember/component/template-only"; const data = {}; - export default setComponentTemplate(precompileTemplate('{{JSON.stringify data}}', { strictMode: true, scope: () => ({ JSON, data }) }), templateOnly()); + export default setComponentTemplate(precompileTemplate('{{${name} data}}', { strictMode: true, scope: () => ({ ${name}, data }) }), templateOnly()); `); + }); + } }); // You might think this would be confusing style, and you'd be correct. But diff --git a/src/scope-locals.ts b/src/scope-locals.ts index 979581b..17773b4 100644 --- a/src/scope-locals.ts +++ b/src/scope-locals.ts @@ -13,25 +13,43 @@ import { astNodeHasBinding } from './hbs-utils'; import { readOnlyArray } from './read-only-array'; /** - * RFC: Pending.... + * RFC: https://github.com/emberjs/rfcs/pull/1070 * - * tl;dr: - * - utilities that begin with uppercase letter - * OR are guaranteed to never be added to glimmer as a keyword (e.g.: globalThis) - * - must not need `new` to invoke + * Criteria for inclusion in this list: * - * Ref: - * - https://tc39.es/ecma262/#sec-global-object - * - https://tc39.es/ecma262/#sec-function-properties-of-the-global-object + * Any of: + * - begins with an uppercase letter + * - guaranteed to never be added to glimmer as a keyword (e.g.: globalThis) + * + * And: + * - must not need new to invoke + * - must not require lifetime management (e.g.: setTimeout) + * - must not be a single-word lower-case API, because of potential collision with future new HTML elements + * - if the API is a function, the return value should not be a promise + * - must be one one of these lists: + * - https://tc39.es/ecma262/#sec-global-object + * - https://tc39.es/ecma262/#sec-function-properties-of-the-global-object + * - https://html.spec.whatwg.org/multipage/nav-history-apis.html#window + * - https://html.spec.whatwg.org/multipage/indices.html#all-interfaces + * - https://html.spec.whatwg.org/multipage/webappapis.html */ -const ALLOWED_GLOBALS = new Set([ +export const ALLOWED_GLOBALS = new Set([ + // //////////////// // namespaces + // //////////////// + // TC39 'globalThis', + 'Atomics', 'JSON', 'Math', - 'Atomics', 'Reflect', - // functions + // WHATWG + 'localStorage', + 'sessionStorage', + // //////////////// + // functions / utilities + // //////////////// + // TC39 'isNaN', 'isFinite', 'parseInt', @@ -40,15 +58,28 @@ const ALLOWED_GLOBALS = new Set([ 'decodeURIComponent', 'encodeURI', 'encodeURIComponent', + // WHATWG + 'postMessage', + 'structuredClone', + // //////////////// // new-less Constructors (still functions) - 'Number', - 'Object', // different behavior from (hash) + // //////////////// + // TC39 'Array', // different behavior from (array) - 'String', 'BigInt', + 'Boolean', 'Date', + 'Number', + 'Object', // different behavior from (hash) + 'String', + // //////////////// // Values + // //////////////// + // TC39 'Infinity', + 'NaN', + // WHATWG + 'isSecureContext', ]); /* From 9306ec1b2eef84f50db7086b7bffb7f79999a84b Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Sun, 16 Mar 2025 15:01:57 -0400 Subject: [PATCH 4/8] Force old glimmer/syntax with node 14 --- .github/workflows/nodejs.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 8b3acbe..a14eb83 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -59,6 +59,8 @@ jobs: node-version: 14 # lockfile format version 9 didn't exist with pnpm 7 - run: pnpm install --no-lockfile + # Newer `@glimmer/syntax`s are not compatible with Node 14 + - run: pnpm add @glimmer/syntax@0.84.3 - run: pnpm test From 00640be180dc807cb9f8be26514e3e357e7e4f6f Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Sun, 16 Mar 2025 15:06:49 -0400 Subject: [PATCH 5/8] Force old glimmer/syntax with node 14 --- .github/workflows/nodejs.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index a14eb83..8f97c10 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -58,9 +58,13 @@ jobs: cache: 'pnpm' node-version: 14 # lockfile format version 9 didn't exist with pnpm 7 - - run: pnpm install --no-lockfile + # We also have to ignore-scripts because we need to change a dep + # because we compile with typescript, and TS will error + # on node 14 if we don't force a downgrade + - run: pnpm install --no-lockfile --ignore-scripts # Newer `@glimmer/syntax`s are not compatible with Node 14 - run: pnpm add @glimmer/syntax@0.84.3 + - run: pnpm build - run: pnpm test From 881a88fd615f89874a649bb8d6d04acd35ba7f77 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Sun, 16 Mar 2025 15:15:55 -0400 Subject: [PATCH 6/8] Set moduleResolution to bundler so that the floating dependencies test passes --- tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tsconfig.json b/tsconfig.json index ccb92e5..9db4100 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -3,6 +3,7 @@ // Compilation Configuration "target": "es2015", "module": "commonjs", + "moduleResolution": "bundler", "inlineSources": true, "inlineSourceMap": true, "declaration": true, From 7e23656bc801105e8588f30618f0a1b84709f1d4 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 18 Mar 2025 15:43:14 -0400 Subject: [PATCH 7/8] Revert unneeded changes --- .github/workflows/nodejs.yml | 8 +------- tsconfig.json | 1 - 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 8f97c10..8b3acbe 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -58,13 +58,7 @@ jobs: cache: 'pnpm' node-version: 14 # lockfile format version 9 didn't exist with pnpm 7 - # We also have to ignore-scripts because we need to change a dep - # because we compile with typescript, and TS will error - # on node 14 if we don't force a downgrade - - run: pnpm install --no-lockfile --ignore-scripts - # Newer `@glimmer/syntax`s are not compatible with Node 14 - - run: pnpm add @glimmer/syntax@0.84.3 - - run: pnpm build + - run: pnpm install --no-lockfile - run: pnpm test diff --git a/tsconfig.json b/tsconfig.json index 9db4100..ccb92e5 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -3,7 +3,6 @@ // Compilation Configuration "target": "es2015", "module": "commonjs", - "moduleResolution": "bundler", "inlineSources": true, "inlineSourceMap": true, "declaration": true, From 0716d0966e3b298fb66331f85e2e75dad275bba8 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 18 Mar 2025 15:44:59 -0400 Subject: [PATCH 8/8] Make test async --- __tests__/tests.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/__tests__/tests.ts b/__tests__/tests.ts index d5314f7..1c11ea0 100644 --- a/__tests__/tests.ts +++ b/__tests__/tests.ts @@ -1904,7 +1904,7 @@ describe('htmlbars-inline-precompile', function () { describe('implements RFC#1070: default globals', function () { for (let name of ALLOWED_GLOBALS) { - it(`${name}: allowed`, function () { + it(`${name}: allowed`, async function () { plugins = [ [ HTMLBarsInlinePrecompile, @@ -1915,7 +1915,7 @@ describe('htmlbars-inline-precompile', function () { ], ]; - let transformed = transform( + let transformed = await transform( `import { template } from '@ember/template-compiler'; const data = {}; export default template('{{${name} data}}', { eval: function() { return eval(arguments[0]) } })