Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Introduce keywords option for precompile #1585

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Introduce keywords option for precompile #1585

merged 2 commits into from
Apr 8, 2024

Conversation

chancancode
Copy link
Contributor

Recently in #1557 we started emitting build time error for strict mode templates containing undefined references. Previously these were not handled until runtime and so we can only emit runtime errors.

However, the previous setup allowed for the host environment to implement additional keywords – these are resolved at runtime with the lookupBuildInHelper and lookupBuiltInModifier hooks, and the error is only emitted if these hooks returned null.

To allow for this possibility, precompile now accept an option for additional strict mode keywords that the host environment is expected to provide.

@chancancode chancancode force-pushed the strict-mode-keywords branch from c0317d8 to 2a19a06 Compare April 4, 2024 23:51
`{{("foo")}}` etc is illegal and already covered by existing tests.

Currently this is checked by the parser (handlebars-node-visitors)
but AST plugins can violate this constraint in theory.
Recently in #1557 we started emitting build time error for strict
mode templates containing undefined references. Previously these
were not handled until runtime and so we can only emit runtime
errors.

However, the previous setup allowed for the host environment to
implement additional keywords – these are resolved at runtime with
the `lookupBuildInHelper` and `lookupBuiltInModifier` hooks, and
the error is only emitted if these hooks returned `null`.

To allow for this possibility, `precompile` now accept an option
for additional strict mode keywords that the host environment is
expected to provide.
@chancancode chancancode force-pushed the strict-mode-keywords branch from 2a19a06 to 025e742 Compare April 4, 2024 23:51
@wycats
Copy link
Contributor

wycats commented Apr 8, 2024

This is a good improvement that gets Glimmer out of the business of knowing Ember's keywords and evolving with Ember. It also allows different versions of Ember to have different keywords without coupling directly to a version of Glimmer.

I'm in favor! :shipit:

@wycats wycats merged commit cd0de78 into main Apr 8, 2024
6 checks passed
@wycats wycats deleted the strict-mode-keywords branch April 8, 2024 20:37
@wycats
Copy link
Contributor

wycats commented Apr 8, 2024

For future readers: this commit is coarse-grained, in the sense that the new API only allows the caller to express "is this a keyword or now".

In reality, there is more granularity in keyword positions in both Ember and Glimmer (block, inline, calls, modifiers, blocks). This means that, for example {{yield}} is a keyword but {{#yield}}{{/yield}} is not a keyword.

Ember and Glimmer aren't fully aligned on the granularity, so I prefer to land this now and unstick the coupling between Ember's keywords and hardcoding in Glimmer.

If someone is reading this comment because the coarseness of this change ended up mattering, now is the time to formalize the different kinds of keywords and expose that granuality via this API (e.g. { inline: [...], block: [...] }, but not exactly this because the whole point is that we have to align Ember and Glimmer and formalize it).

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants