search

glimmerjs / glimmer-vm

MIT License
1.13k stars 191 forks source link

Introduce `keywords` option for `precompile` #1585

Closed chancancode closed 7 months ago

chancancode commented 7 months ago

Recently in https://github.com/glimmerjs/glimmer-vm/pull/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.

wycats commented 7 months ago

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 commented 7 months ago

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).