thefrontside / ember-let

Create variable bindings inside your handlebars templates
MIT License
52 stars 16 forks source link

Rewrite inline let to have lexical scope #12

Closed mmun closed 7 years ago

mmun commented 7 years ago

EDIT: The syntax of {{let}} and {{#let}} has changed since this PR.

This PR reimplements inline let as an HTMLBars AST transform. There is no notion of inline let at runtime. It works by rewriting the let statements as nested blocks. For example,

<div>
  {{let a 1 b 2}}
  {{let a 11 b 12 c (add a b)}}
  {{a}} {{b}} {{c}}
  <div>
    {{let a 5}}
    {{a}} {{b}} {{c}}
  </div>
  {{let a (add a 10)}}
  {{a}} {{b}} {{c}}
</div>

is transformed to (excluding whitespace changes to make it easier to read):

<div>
  {{#let 1 2 as |a b|}}
    {{#let 11 12 (add a b) as |a b c|}}
      {{a}} {{b}} {{c}}
      <div>
        {{#let 5 as |a|}}
          {{a}} {{b}} {{c}}
        {{/let}}
      </div>
      {{#let (add a 10) as |a|}}
        {{a}} {{b}} {{c}}
      {{/let}}
    {{/let}}
  {{/let}}
</div>

Both templates generate the DOM

<div>
  11 12 3
  <div>
    5 12 3
  </div>
  21 12 3
</div>
taras commented 7 years ago

@mmun thank you!

Ember 1.13 test is failing. In the current test suite, we're not testing inline let in 1.13.

Should we exclude the AST transform from for Ember 1.13 or is it possible to make it work with 1.13?

mmun commented 7 years ago

Should be possible. :)

EDIT: Actually, it would be a pain to get it working in 1.13 so I disable the transform below 2.0.0.

mmun commented 7 years ago

I've changed the example in the PR message to demonstrate that bindings are created in parallel rather than series (E.g. the semantics are like LISP's let rather than let*).

Robdel12 commented 7 years ago

This is pretty awesome 🎉

fivetanley commented 7 years ago

is this glimmer 2 compatible?

cowboyd commented 7 years ago

An excellent question, is there a way that we can test it?

Robdel12 commented 7 years ago

https://github.com/emberjs/ember.js/pull/14156 looks like we can build vs canary?

cowboyd commented 7 years ago

So looks like it's going to require some work to get it working on Glimmer. We can track it with #14, but I don't think it should hold this up getting merged.

cowboyd commented 7 years ago

I can't begin to express how happy this makes me. A truly lexical inline form is almost bringing tears to my eyes y'all. Not even joking. Thanks @mmun and @rwjblue making this happen.

taras commented 7 years ago

Absolutely, thanks guys so much for helping us make this work.

mmun commented 7 years ago

@cowboyd I have a follow up PR soon that adds the warning.