Skip to content

Pull in Glimmer VM #20896

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

Draft
wants to merge 3,833 commits into
base: main
Choose a base branch
from
Draft

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Apr 25, 2025

chancancode and others added 30 commits February 25, 2024 22:41
The original commit has a few issues:

- Type safety
- Added a new AST node, which is unnecessary since we already have
  `VarHead`, but is also a breaking change and is kind of breakage
  we have been careful to avoid in this area thus far
- The parsing is robust against a few classes of edge cases

This rework the same feature (on the `Block`/`BlockStatement` side)
and adds more test coverage for the feature. Also renamed the field
to `block.params`.
improve arguments error detection for inspector
The previous/existing "parsing" code tries to piggy back on letting
the HTML tokenizer/parser parse out the block params syntax into
attribute nodes and tries to recover the information after the fact

This is fundamentally broken and suffers from some wild edge cases
like:

```
<div as="" |foo="bar|>...</div> // => <div as |foo|>...</div>
```

This is especially problematic in light of wanting to retain the
source spans of the block params nodes.

This commit rework the block params parsing and integrate it into
the normal parsing pass in the appropiate time.
Provide source spans for the open and close tags without adding new
AST nodes, also parse the element's tag name into a path which has
the required source span.
It turns out to be important that AST plugins can mutate the list
of local variables in the root `Template` node. Instead, use *that*
list of local variables as the ultimate source of truth after AST
plugin ran.
The rename is motivated by:

1. It's confusing for the root node to have "block params"
2. All other AST nodes that has `blockParams` also has a `params`
   field that exposes the source location as `VarHead`s

There is some uncertainty in:

1. Whether there are a lot of existing AST consumers that uses it
   (shouldn't be a big deal in itself, as it's just a deprecation)
2. Whether the direction of using `Template.locals` as the source
   of truth for outside local variables will ultimately prevail

When we have more certainty on those and if this rename is still
desired, it should be possible to revert this commit
…nal-arguments-when-modifiers-update

Don't infinitely duplicate positional arguments when modifiers update
My learnings from hacking on this in inspector
https://github.com/emberjs/ember-inspector/pull/2549/files

Adding modifiers is also easy, will add it in another pr
…ender-tree

add in element to debug render tree
This was dropped on Ember side a while back, just never cleaned up

Ironically this probably meant when Ember dropped the deprecation/
error, nothing was actually stopping `{{#with}}` from anymore, so
perhaps for a while we have actually _re-introduced_ support for
the syntax for some time?

Extracted from emberjs#1557
[CLEANUP] Drop support for `{{#with}}` keyword
…-render-tree

add modifiers to debug render tree
Removing support for implicit this fallback {{foo}} -> {{this.foo}}

When a strict mode template references an undefined variable, it
really should produce a compile-time error, arguably that is the
point of the feature.

However, it seems like previously a lot of these errors ended up
propagating and is actually handled by the runtime compiler.

The original/primary goal here started out as cleaning up all the
code for implicit this fallback, but as I delete code it eventually
revaled all the places where strict mode was inappropiately using
the same infrastructure ("unresolved free variables", which isn't
really a sensible thing).

Also:

* Removes "deprecated call" @foo={{bar}}
* Removes unused opcodes
to all package.json files which do not have `"private": true`
[ENHANCEMENT+CLEANUP] Better compile time errors for strict mode
…rjs#1580)

* Remove deprecation for setting hash properties (from years ago)

* Remove tests which are now irrelevant
…#1583)

The refactor in emberjs#1568 slightly changed the semantics of `path.parts`
in that it didn't previously include `this` or the leading `@`. This
commit restores the previous semantics.
…uleResolution=node10

Re-add support for tsconfig's moduleResolution=node10
…uleResolution=node10

Re-add support for tsconfig's moduleResolution=node10
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.

6 participants