Skip to content

Conversation

@NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Jan 14, 2026

2026-01-20 post-review notes:


https://rfcs.emberjs.com/id/0997-make-on-built-in

We already have tests for importing on.

Of note, our compiler is super weird, and made this way more difficult than it should have been.

We had an unused option, lexicalScope, which I think is how we well the compiler what shadows keywords. Hopefully over time we can fix all this and have a simple object of references to use instead of this array/function silliness.

This PR lays the foundation for all future modifiers/helpers being used as "keywords"

(which are all going to be more tests than implementation (we're set up for each new utility to basically be one line each))

Also in this PR is an internal upgrade for our smoke-test app(s), because I was hitting the error where emberAppSatisfies was missing due to out of date deps.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2026

Estimated Asset Sizes

Diff

--- main/out.txt	2026-01-16 20:37:26.000000000 +0000
+++ pr/./pr-21101513329/out.txt	2026-01-17 22:02:26.000000000 +0000
@@ -1,37 +1,37 @@
-╔═══════╤══════════╤══════════╗
-║       │ Min      │ Gzip     ║
-╟───────┼──────────┼──────────╢
-║ Total │ 352.2 KB │ 203.5 KB ║
-╚═══════╧══════════╧══════════╝
+╔═══════╤═══════════╤═══════════╗
+║       │ Min       │ Gzip      ║
+╟───────┼───────────┼───────────╢
+║ Total │ 352.28 KB │ 203.77 KB ║
+╚═══════╧═══════════╧═══════════╝
 
 ╔══════════════════════╤═══════════╤═══════════╗
 ║ @ember/*             │ Min       │ Gzip      ║
 ╟──────────────────────┼───────────┼───────────╢
-║ Total                │ 313.81 KB │ 181.76 KB ║
+║ Total                │ 313.86 KB │ 181.98 KB ║
 ╟──────────────────────┼───────────┼───────────╢
-║ -internals           │ 36.61 KB  │ 25.99 KB  ║
-║ application          │ 13.24 KB  │ 7.99 KB   ║
-║ array                │ 13.05 KB  │ 7.54 KB   ║
+║ -internals           │ 36.56 KB  │ 25.92 KB  ║
+║ application          │ 13.24 KB  │ 8.01 KB   ║
+║ array                │ 13 KB     │ 7.55 KB   ║
 ║ canary-features      │ 304 B     │ 419 B     ║
-║ component            │ 2.05 KB   │ 1.61 KB   ║
-║ controller           │ 1.96 KB   │ 1.45 KB   ║
-║ debug                │ 11.73 KB  │ 8.14 KB   ║
+║ component            │ 2.09 KB   │ 1.63 KB   ║
+║ controller           │ 1.96 KB   │ 1.41 KB   ║
+║ debug                │ 11.73 KB  │ 8.15 KB   ║
 ║ deprecated-features  │ 31 B      │ 77 B      ║
 ║ destroyable          │ 561 B     │ 383 B     ║
 ║ enumerable           │ 259 B     │ 387 B     ║
-║ helper               │ 1.08 KB   │ 802 B     ║
+║ helper               │ 1.08 KB   │ 829 B     ║
 ║ instrumentation      │ 2.43 KB   │ 1.78 KB   ║
-║ modifier             │ 1.22 KB   │ 978 B     ║
-║ object               │ 35.98 KB  │ 22.1 KB   ║
+║ modifier             │ 1.27 KB   │ 1 KB      ║
+║ object               │ 35.98 KB  │ 22.13 KB  ║
 ║ owner                │ 159 B     │ 178 B     ║
-║ renderer             │ 630 B     │ 471 B     ║
-║ routing              │ 59.37 KB  │ 33.93 KB  ║
-║ runloop              │ 2.36 KB   │ 1.5 KB    ║
+║ renderer             │ 630 B     │ 480 B     ║
+║ routing              │ 59.37 KB  │ 34.21 KB  ║
+║ runloop              │ 2.36 KB   │ 1.45 KB   ║
 ║ service              │ 1 KB      │ 858 B     ║
-║ template             │ 654 B     │ 543 B     ║
+║ template             │ 654 B     │ 518 B     ║
 ║ template-compilation │ 429 B     │ 366 B     ║
-║ template-compiler    │ 123.33 KB │ 59.68 KB  ║
-║ template-factory     │ 370 B     │ 351 B     ║
+║ template-compiler    │ 123.4 KB  │ 59.61 KB  ║
+║ template-factory     │ 370 B     │ 382 B     ║
 ║ test                 │ 923 B     │ 627 B     ║
 ║ utils                │ 4.11 KB   │ 3.63 KB   ║
 ║ version              │ 55 B      │ 131 B     ║
@@ -40,19 +40,19 @@
 ╔═════════════════╤══════════╤══════════╗
 ║ @glimmer/*      │ Min      │ Gzip     ║
 ╟─────────────────┼──────────┼──────────╢
-║ Total           │ 38.38 KB │ 21.74 KB ║
+║ Total           │ 38.43 KB │ 21.79 KB ║
 ╟─────────────────┼──────────┼──────────╢
 ║ destroyable     │ 2.78 KB  │ 1.38 KB  ║
 ║ encoder         │ 81 B     │ 171 B    ║
 ║ env             │ 38 B     │ 87 B     ║
 ║ global-context  │ 886 B    │ 545 B    ║
-║ manager         │ 977 B    │ 627 B    ║
-║ node            │ 175 B    │ 243 B    ║
-║ opcode-compiler │ 1.11 KB  │ 905 B    ║
+║ manager         │ 921 B    │ 595 B    ║
+║ node            │ 175 B    │ 238 B    ║
+║ opcode-compiler │ 1.11 KB  │ 922 B    ║
 ║ owner           │ 159 B    │ 202 B    ║
-║ program         │ 252 B    │ 333 B    ║
+║ program         │ 252 B    │ 296 B    ║
 ║ reference       │ 548 B    │ 544 B    ║
-║ runtime         │ 10.32 KB │ 5.19 KB  ║
+║ runtime         │ 10.42 KB │ 5.3 KB   ║
 ║ tracking        │ 1.34 KB  │ 1.18 KB  ║
 ║ util            │ 1.94 KB  │ 1.6 KB   ║
 ║ validator       │ 15.53 KB │ 6.9 KB   ║

Details

This PRmain
╔═══════╤═══════════╤═══════════╗
║       │ Min       │ Gzip      ║
╟───────┼───────────┼───────────╢
║ Total │ 352.28 KB │ 203.77 KB ║
╚═══════╧═══════════╧═══════════╝

╔══════════════════════╤═══════════╤═══════════╗
║ @ember/*             │ Min       │ Gzip      ║
╟──────────────────────┼───────────┼───────────╢
║ Total                │ 313.86 KB │ 181.98 KB ║
╟──────────────────────┼───────────┼───────────╢
║ -internals           │ 36.56 KB  │ 25.92 KB  ║
║ application          │ 13.24 KB  │ 8.01 KB   ║
║ array                │ 13 KB     │ 7.55 KB   ║
║ canary-features      │ 304 B     │ 419 B     ║
║ component            │ 2.09 KB   │ 1.63 KB   ║
║ controller           │ 1.96 KB   │ 1.41 KB   ║
║ debug                │ 11.73 KB  │ 8.15 KB   ║
║ deprecated-features  │ 31 B      │ 77 B      ║
║ destroyable          │ 561 B     │ 383 B     ║
║ enumerable           │ 259 B     │ 387 B     ║
║ helper               │ 1.08 KB   │ 829 B     ║
║ instrumentation      │ 2.43 KB   │ 1.78 KB   ║
║ modifier             │ 1.27 KB   │ 1 KB      ║
║ object               │ 35.98 KB  │ 22.13 KB  ║
║ owner                │ 159 B     │ 178 B     ║
║ renderer             │ 630 B     │ 480 B     ║
║ routing              │ 59.37 KB  │ 34.21 KB  ║
║ runloop              │ 2.36 KB   │ 1.45 KB   ║
║ service              │ 1 KB      │ 858 B     ║
║ template             │ 654 B     │ 518 B     ║
║ template-compilation │ 429 B     │ 366 B     ║
║ template-compiler    │ 123.4 KB  │ 59.61 KB  ║
║ template-factory     │ 370 B     │ 382 B     ║
║ test                 │ 923 B     │ 627 B     ║
║ utils                │ 4.11 KB   │ 3.63 KB   ║
║ version              │ 55 B      │ 131 B     ║
╚══════════════════════╧═══════════╧═══════════╝

╔═════════════════╤══════════╤══════════╗
║ @glimmer/*      │ Min      │ Gzip     ║
╟─────────────────┼──────────┼──────────╢
║ Total           │ 38.43 KB │ 21.79 KB ║
╟─────────────────┼──────────┼──────────╢
║ destroyable     │ 2.78 KB  │ 1.38 KB  ║
║ encoder         │ 81 B     │ 171 B    ║
║ env             │ 38 B     │ 87 B     ║
║ global-context  │ 886 B    │ 545 B    ║
║ manager         │ 921 B    │ 595 B    ║
║ node            │ 175 B    │ 238 B    ║
║ opcode-compiler │ 1.11 KB  │ 922 B    ║
║ owner           │ 159 B    │ 202 B    ║
║ program         │ 252 B    │ 296 B    ║
║ reference       │ 548 B    │ 544 B    ║
║ runtime         │ 10.42 KB │ 5.3 KB   ║
║ tracking        │ 1.34 KB  │ 1.18 KB  ║
║ util            │ 1.94 KB  │ 1.6 KB   ║
║ validator       │ 15.53 KB │ 6.9 KB   ║
║ vm              │ 495 B    │ 569 B    ║
║ wire-format     │ 1.84 KB  │ 1.35 KB  ║
╚═════════════════╧══════════╧══════════╝
╔═══════╤══════════╤══════════╗
║       │ Min      │ Gzip     ║
╟───────┼──────────┼──────────╢
║ Total │ 352.2 KB │ 203.5 KB ║
╚═══════╧══════════╧══════════╝

╔══════════════════════╤═══════════╤═══════════╗
║ @ember/*             │ Min       │ Gzip      ║
╟──────────────────────┼───────────┼───────────╢
║ Total                │ 313.81 KB │ 181.76 KB ║
╟──────────────────────┼───────────┼───────────╢
║ -internals           │ 36.61 KB  │ 25.99 KB  ║
║ application          │ 13.24 KB  │ 7.99 KB   ║
║ array                │ 13.05 KB  │ 7.54 KB   ║
║ canary-features      │ 304 B     │ 419 B     ║
║ component            │ 2.05 KB   │ 1.61 KB   ║
║ controller           │ 1.96 KB   │ 1.45 KB   ║
║ debug                │ 11.73 KB  │ 8.14 KB   ║
║ deprecated-features  │ 31 B      │ 77 B      ║
║ destroyable          │ 561 B     │ 383 B     ║
║ enumerable           │ 259 B     │ 387 B     ║
║ helper               │ 1.08 KB   │ 802 B     ║
║ instrumentation      │ 2.43 KB   │ 1.78 KB   ║
║ modifier             │ 1.22 KB   │ 978 B     ║
║ object               │ 35.98 KB  │ 22.1 KB   ║
║ owner                │ 159 B     │ 178 B     ║
║ renderer             │ 630 B     │ 471 B     ║
║ routing              │ 59.37 KB  │ 33.93 KB  ║
║ runloop              │ 2.36 KB   │ 1.5 KB    ║
║ service              │ 1 KB      │ 858 B     ║
║ template             │ 654 B     │ 543 B     ║
║ template-compilation │ 429 B     │ 366 B     ║
║ template-compiler    │ 123.33 KB │ 59.68 KB  ║
║ template-factory     │ 370 B     │ 351 B     ║
║ test                 │ 923 B     │ 627 B     ║
║ utils                │ 4.11 KB   │ 3.63 KB   ║
║ version              │ 55 B      │ 131 B     ║
╚══════════════════════╧═══════════╧═══════════╝

╔═════════════════╤══════════╤══════════╗
║ @glimmer/*      │ Min      │ Gzip     ║
╟─────────────────┼──────────┼──────────╢
║ Total           │ 38.38 KB │ 21.74 KB ║
╟─────────────────┼──────────┼──────────╢
║ destroyable     │ 2.78 KB  │ 1.38 KB  ║
║ encoder         │ 81 B     │ 171 B    ║
║ env             │ 38 B     │ 87 B     ║
║ global-context  │ 886 B    │ 545 B    ║
║ manager         │ 977 B    │ 627 B    ║
║ node            │ 175 B    │ 243 B    ║
║ opcode-compiler │ 1.11 KB  │ 905 B    ║
║ owner           │ 159 B    │ 202 B    ║
║ program         │ 252 B    │ 333 B    ║
║ reference       │ 548 B    │ 544 B    ║
║ runtime         │ 10.32 KB │ 5.19 KB  ║
║ tracking        │ 1.34 KB  │ 1.18 KB  ║
║ util            │ 1.94 KB  │ 1.6 KB   ║
║ validator       │ 15.53 KB │ 6.9 KB   ║
║ vm              │ 495 B    │ 569 B    ║
║ wire-format     │ 1.84 KB  │ 1.35 KB  ║
╚═════════════════╧══════════╧══════════╝

}
}

const modifierKeywords: Record<string, object> = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't the same way other things are looked up (which is done via a resolver) -- I don't think we should do that -- this is static information.

If anyone thinks I should do this differently, lemme know

@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review January 14, 2026 23:58
@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Jan 15, 2026

so close -- I have smoke tests to fix.

Probably means I made a cycle with importing from @glimmer/runtime from the opcode-builder. 🤔

@NullVoxPopuli NullVoxPopuli marked this pull request as draft January 16, 2026 00:10
@NullVoxPopuli
Copy link
Contributor Author

back to draft for now.

@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review January 17, 2026 22:01
assert.step('success');
};

const compiled = template('<button {{on "click" handleClick}}>Click</button>', {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runtime compiler used here so I could debug the compiler in the browser

options: PrecompileOptions | PrecompileOptionsWithLexicalScope = defaultOptions
): [block: SerializedTemplateBlock, usedLocals: string[]] {
const source = new src.Source(string ?? '', options.meta?.moduleName);
const [ast, locals] = normalize(source, { lexicalScope: () => false, ...options });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it feels like lexicalScope: () => false is an unfinished plan -- feels like it was meant to pair with strict mode, but was never used (until now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants