Skip to content
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

Improve lexical scope #1421

Merged
merged 2 commits into from
May 4, 2023
Merged

Improve lexical scope #1421

merged 2 commits into from
May 4, 2023

Conversation

wycats
Copy link
Contributor

@wycats wycats commented May 4, 2023

Improved lexical scope

This is an initial implementation of an improved API for compiling a
template with lexical scope.

Instead of passing a list of locals to the precompileTemplate function,
you can pass a function that takes a variable name and returns a boolean
to indicate whether the variable is in scope.

Details

This fixes an inconsistency with how HTML tags and keywords
behave when they refer to a Handlebars local variable vs. when they
refer to a JavaScript lexical variable.

const component = <template><div>{{yield}}</div></template>;

<template>
  <component />
</template>

After this change, the <component> invocation correctly refers to the
surrounding scope variable, even though component is a keyword.

This makes it consistent with:

const variable = <template><div>{{yield}}</div></template>;

<template>
{{#let variable as |component|}}
  <component />
{{/let}}
</template>

These semantics are an important future-proofing measure: they allow us
to add additional keywords in the future, since local variables (either
lexical JavaScript variables of Handlebars variables) will win over the
newly defined keywords.

If we allowed keywords to win over lexical JavaScript variables, every
new keyword would be a breaking change. The semantics we used for
as |div| and as |component| were carefully designed to avoid this
problem, and this change helps us implement the same behavior for
lexical JavaScript variables.

@wycats wycats force-pushed the feature/template-locals-v2 branch 3 times, most recently from 9bab6b2 to b31c79c Compare May 4, 2023 03:14
wycats added 2 commits May 3, 2023 20:26
This is an initial implementation of an improved API for compiling a
template with lexical scope.

Instead of passing a list of locals to the precompileTemplate function,
you can pass a function that takes a variable name and returns a boolean
to indicate whether the variable is in scope.

Importantly, this fixes an inconsistency with how HTML tags and keywords
behave when they refer to a Handlebars local variable vs. when they
refer to a JavaScript lexical variable.

```jsx
const component = <template><div>{{yield}}</div></template>;

<template>
  <component />
</template>
```

After this change, the `<component>` invocation correctly refers to the
surrounding scope variable, even though `component` is a keyword.

This makes it consistent with:

```jsx
const variable = <template><div>{{yield}}</div></template>;

<template>
{{#let variable as |component|}}
  <component />
{{/let}}
</template>
```

These semantics are an important future-proofing measure: they allow us
to add additional keywords in the future, since local variables (either
lexical JavaScript variables of Handlebars variables) will win over the
newly defined keywords.

If we allowed keywords to win over lexical JavaScript variables, every
new keyword would be a breaking change. The semantics we used for
`as |div|` and `as |component|` were carefully designed to avoid this
problem, and this change helps us implement the same behavior for
lexical JavaScript variables.
@wycats wycats force-pushed the feature/template-locals-v2 branch from 6cdcda5 to 3d4f102 Compare May 4, 2023 03:28
@wycats wycats marked this pull request as ready for review May 4, 2023 03:32
@wycats wycats merged commit 82a81f7 into master May 4, 2023
@wycats wycats deleted the feature/template-locals-v2 branch May 4, 2023 03:37
@ef4
Copy link
Contributor

ef4 commented May 30, 2023

As soon as this is in a release I'm ready to use it.

@NullVoxPopuli
Copy link
Contributor

should be in 0.85.3

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

Successfully merging this pull request may close these issues.

3 participants