-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
RFC#997 - {{on}} as keyword #21055
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
RFC#997 - {{on}} as keyword #21055
Conversation
Estimated Asset SizesDiff --- 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
|
| } | ||
| } | ||
|
|
||
| const modifierKeywords: Record<string, object> = { |
There was a problem hiding this comment.
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
|
so close -- I have smoke tests to fix. Probably means I made a cycle with importing from |
|
back to draft for now. |
78039b0 to
7f54911
Compare
7f54911 to
3d16f66
Compare
This reverts commit bf94a6c.
| assert.step('success'); | ||
| }; | ||
|
|
||
| const compiled = template('<button {{on "click" handleClick}}>Click</button>', { |
There was a problem hiding this comment.
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 }); |
There was a problem hiding this comment.
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)
2026-01-20 post-review notes:
lexicalScopeis only for the runtime compilerevalhttps://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"
fn(RFC #998),hash(RFC #999),array(RFC #1000)and,or,not- RFC #562lt,lte,gt,gte- RFC #561eq,neq- RFC #560element- RFC #389(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
emberAppSatisfieswas missing due to out of date deps.