Open
Conversation
NullVoxPopuli
changed the title
Contributor
Estimated Asset SizesDiff Details
|
Contributor
Author
|
Edit: solved Currently failing in Safari and Firefox 🤔 This repros locally |
NullVoxPopuli
changed the title
NullVoxPopuli
commented
Jan 28, 2026
| if (options.eval) { | ||
| return options.eval; | ||
| } else { | ||
| const scope = options.scope?.(); |
Contributor
Author
There was a problem hiding this comment.
@ef4 this is why I think we need to change the compiled JSON format to allow objects for scope.
Because the runtime compiler now eagerly consumes all args upon setting up the component to render.
This is incorrect, because args should not be consumed until rendered.
NullVoxPopuli
added a commit
that referenced
this pull request
Jan 28, 2026
NullVoxPopuli
force-pushed
the
nvp/on-as-keyword-tremplate-the-transform-way
branch
from
357a531 to
afde876
Compare
NullVoxPopuli
force-pushed
the
nvp/on-as-keyword-tremplate-the-transform-way
branch
from
afde876 to
8f84f97
Compare
NullVoxPopuli
commented
Jan 30, 2026
| return evaluator; | ||
| } | ||
|
|
||
| scope = Object.assign({}, keywords, scope); |
Contributor
Author
There was a problem hiding this comment.
these need to be mixed in here when using "explicit scope"
Contributor
Author
There was a problem hiding this comment.
a demo of what happens when we remove: #21073
not ok 14 Chrome 144.0 - [5 ms] - [integration] jit :: keyword modifier: on (runtime): explicit scope
---
stack: >
ReferenceError: on is not defined
NullVoxPopuli
commented
Jan 30, 2026
| return component; | ||
| } | ||
|
|
||
| Object.assign(template, keywords); |
Contributor
Author
There was a problem hiding this comment.
we have to have a known reference for "implicit scope" to use -- becauseu we can't change imports.
the only alternative to this is maybe having a static list of things on globalThis, but then we can't have multiple ember versions loaded at once
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alternate implementation to #21055
https://rfcs.emberjs.com/id/0997-make-on-built-in
We already have tests for importing on.
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.