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

let it be: bind names to values in templates #200

Closed
wants to merge 3 commits into from

Conversation

cowboyd
Copy link

@cowboyd cowboyd commented Jan 15, 2017

@cowboyd cowboyd changed the title Bind names to values in templates with thelet keyword let it be: bind names to values in templates Jan 15, 2017
what exactly it does without simultaneously holding both the `.hbs`
and `.js` files together and continuously merging them with your
mind. The lack of this overhead is one of the commonly cited strengths
of JSX over Handlebars.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've always considered this to be a strength of Ember - not mixing invocation and implementation both in the template layer but keeping them apart. I generally think of the template context as an API for the template. When you use an API you don't want to deal with all of its internals but rather use it by invoking properly named properties and actions and assume it does the right thing under the hood. That takes a lot of complexity out of the template layer which is actually a good thing IMO.

I also gave a talk about the topic at last year's Emberfest: https://speakerdeck.com/marcoow/templates-and-logic-in-ember - hope videos will be up soon.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually read your slides very shortly after your talk, and I look forward watching the video!

requirement to explicitly enumerate dependencies. If the computation
of a property has incorrectly specified dependent keys it will break
in mysterious ways that usually involve a sudden and hard-to-reproduce
lack of reactivity to changing inputs.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better approach to fix this is to make CPs easier. One way of doing so would be to come up with a special kind of CP that automatically tracks its dependencies. Doing that should (more or less) easily be possible, I just never found the time yet to build a prototype.

Also I think that computed properties macros can help make using CPs much easier for a lot of people and addons like https://github.com/kellyselden/ember-awesome-macros are doing a great job helping with that. /cc @kellyselden

Copy link
Author

@cowboyd cowboyd Jan 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better approach to fix this is to make CPs easier. One way of doing so would be to come up with a special kind of CP that automatically tracks its dependencies. Doing that should (more or less) easily be possible, I just never found the time yet to build a prototype.

You can not do this without code that comprehends the JavaScript AST of the function representing your property. That would be very cool, but would also come with its own complexities. Handlebars templates already comprehend the dependency flow at a syntax level and so we can lean on them for free.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can actuylly do. that by hooking into get when running the CP function. You don't actually depend on all of the dependent keys all the time - actually you don't depend on anything that is not get'd when computing the CP so e.g. you dont depend on anything that's accessed within if (this.get('x')) {as long asxisfalse`.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible, certainly, but is more and deeper magic the answer? Ember already plagued by problems like having the meaning of _super be context sensitive

Copy link

@marcoow marcoow May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We built https://github.com/simplabs/ember-auto-computed as a proof of concept for the approach. Also I think @wycats implemented something similar for Glimmer.js' tracked properties.


This solves the problem of proximity by placing the derivation of a
value right next to where it's used. There's no question about how the
disabled attribute is computed. It also solves the dependency problem
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue the fact that you don't have to know how the value is computed when using a CP is a good thing actually.

Copy link
Author

@cowboyd cowboyd Jan 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this statement: Not needing to understand the computation is a good thing, and that is what assigning a name to a computation is about. You can think about the name, not the computation.

This is actually one of the problems I have with composable helpers:

<div class={{if (or isLoading isWaiting) 'pending'}}></div>

You can't look at this code and not understand the pending class computation because it's mashed right up in your face. You don't have the option of deferring understanding until a time of your choosing, and that's not good.

The problem, is that when you do want to understand the computation, then it's time to do a name lookup and you are forced to search in a different file, controller, component, etc...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem, is that when you do want to understand the computation, then it's time to do a name lookup and you are forced to search in a different file, controller, component, etc...

I honestly don't think this is an actual problem that people have - especially since there's only ever one JS file per template that you'd have to look into.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem, is that when you do want to understand the computation, then it's time to do a name lookup and you are forced to search in a different file, controller, component, etc...

that's also something that can be improved by working on better tools that allow references in templates to be resolved

value right next to where it's used. There's no question about how the
disabled attribute is computed. It also solves the dependency problem
because the dependencies of an expression are implicity enumerated
merely through the act of expressing it.
Copy link

@marcoow marcoow Jan 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helpers don't alway behave as expected though. While they observe the values of their arguments they don't observer the value of any properties on the arguments. E.g.

<div>{{fullName user}}</div>

where fullName is

function([user]) {
  return `${user.firstName} ${user.lastName}`;
}

will not update when the user's firstName or lastName properties change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this is as a problem generally with using shared mutable state. Unfortunately, this is still the common Ember practice, and in practice what you see all the time is users only specifying the base object and not attributes in their computed properties.

The way we write apps today with an immutable style, the fullName helper you describe works just fine, because a change to user.firstName means that the reference to user changes as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which is to say that I think this is orthogonal to the scope of this rfc

{{/let}}
</div>
a + b = {{sum a b}} <!-- 3 -->
{{/let}}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm honestly worried that we'd start seeing templates like this in client projects :trollface:

@simonihmig
Copy link
Contributor

I think the overarching question as @marcoow has already pointed out is how much logic you want to see in your templates, or broadly speaking how the relation of template and component/controller should look like, in terms of responsibilities and concerns. And I am not sure the Ember community has found a consensus there (yet).

On one side addons like your ember-let or ember-composable-helper giving you the power to add more logic into the template. But I for myself am more on the logic-less side, and see the separation of template and JS context as a real strength there compared to things like JSX for example (having a PHP background, I have seen some bad examples of mixing concerns, be assured! 🤒). Also, the examples somehow feel much more imperative to me, rather than declarative, which I loved.

Also don't know what implications that would have on the rendering engine, seems there would be quite some managing of state required (e.g. you would not be able to just rerender the nested <div> in your example without reevaluating the outer scope, or tracking the state of it). But others than me will certainly be in a better position to judge this...

I think there is already some very reasonable policy in place in the Ember community of first trying new things out in addon-land, and when this has been explored and proven and became popular this way, then this can be integrated into core. So, if we see a broad adoption of ember-let or other addons in this space, we could reopen this. But honestly, no offense intended, I won't be on that side of the game...

@cowboyd
Copy link
Author

cowboyd commented Jan 16, 2017

Also, the examples somehow feel much more imperative to me, rather than declarative, which I loved.

@simonihmig I'm not sure I follow. Imperative constructs just don't exist in handlebars, and I think we all can agree that's a strength.

@cowboyd
Copy link
Author

cowboyd commented Jan 16, 2017

Also don't know what implications that would have on the rendering engine, seems there would be quite some managing of state required (e.g. you would not be able to just rerender the nested

in your example without reevaluating the outer scope, or tracking the state of it). But others than me will certainly be in a better position to judge this...

I don't think there will be any need to change the rendering engine. let is really just the same as with except it renders its block for all values, not just "truthy" ones.

@marcoow @simonihmig As for best practice, I'm not prescribing that users should always declare their computations in their templates, or that you should not use computed properties anywhere, but I think it's also important to acknowledge that the persistent popularity of addons like ember-composable-helpers demonstrate that a large proportion of Ember developers already feel the pressures I'm describing and have for some time

Many of the problems with using composable helpers today stems from the fact that they are used to generate large, anonymous, and therefore opaque expressions. Not wanting those in your code base is a good thing, and so I see let and CPs as cooperative tools, not in conflict.

@simonihmig
Copy link
Contributor

I'm not sure I follow. Imperative constructs just don't exist in handlebars, and I think we all can agree that's a strength.

I was referring to your inline style, that kind of gave me a feeling of imperative code, in the sense that the order of statements is of importance. It could give the impression that the template is kind of executed line by line from top to bottom. I think this implicit scope of the inline style could lead to confusion. I did not have this issue with the block style. But maybe it's just me...

@simonihmig
Copy link
Contributor

I don't think there will be any need to change the rendering engine. let is really just the same as with except it renders its block for all values, not just "truthy" ones.

After rereading your proposal, do I understand you correctly that you want this...

{{let a=1 b=2}}
a + b = {{sum a b}}

to be implicitly transformed to this one:

{{#let 1 2 as |a b|}}
  a + b = {{sum a b}}
{{/let}}

probably at build time? So the latter is the only thing that Glimmer2 or whatever the rendering engine is will see? Then I can follow you. I thought the rendering engine would have to manage the implicitly created scope somehow...

@rwjblue
Copy link
Member

rwjblue commented Jan 16, 2017

@simonihmig - Yes, and FWIW that is how ember-let works today (inline let is transformed to block form at build time).

@averydev
Copy link

averydev commented Jan 16, 2017

I am a strong believer in two things regarding templates and separation of concerns:

  1. Complex logic should be kept out of templates as much as possible. Of course it's just a simple example, but in the fullName instance, it's actually a great illustration of how fullName should just be a CP on the User model ;)
  2. There is a difference to me between the data / logic that backs the view layer, and the data that is required to support just the layout of the view layer. Muddying up the backing js with an extravagant amount of layout specific CP's is just as bad as muddying up the template imo.

I am a happy user of ember-truth-helpers because in many cases, the result is less + simpler code overall. If the effect of less code and more clarity, then bending the rules on the logiclessness of templates is well worth it.

I want to throw out there an element of the Liquid templating language which I'm quite fond of which might be worth taking a peek at: assign.

I've used Liquid pretty extensively, and assign is a tool that is one of its most powerful features Liquid has. It can also be brutally abused. In Liquid, assigned variables are available to anything in that template, but aren't available to any sub components unless they are passed to that component explicitly. I'm not clear on the ins-and-outs of the scope of those assignments, but that's the general idea.

I'm interested to see the direction this RFC goes... I've already learned about some pretty fantastic addons i wasn't familiar with 💯

@lolmaus
Copy link

lolmaus commented Jan 17, 2017

For users looking for the very basic functionality that's described here, here's a method that already works:

{{#with (hash
 one   = "One"
 two   = "Two"
 three = "Three"
) as |h|}}
  <p>I've been to {{h.one}} beach, {{h.two}} coffee shops and {{h.three}} bars.
{{/with}}

I think this should be added to the alternatives section.

@cowboyd
Copy link
Author

cowboyd commented Jan 17, 2017

@lolmaus The current alternatives section has this to say:

Doing nothing is certainly one approach. with is almost let, and in most cases works just well enough, but the caveats it contains make it discouraging of the use-cases that let unlocks.

I reworked it to include you example. Does it seem more clear now?

@cowboyd cowboyd force-pushed the let-keyword branch 2 times, most recently from e7b2077 to 87fa909 Compare January 17, 2017 03:18
@lolmaus
Copy link

lolmaus commented Jan 17, 2017

@cowboyd Thank you. In Alternatives, you say "see caveat_s_ in Detailed design", but Detail design contains only one caveat. You literally say "The only difference".

Being very explicit about this will make it easier to understand the value of this RPC. I can now see three benefits:

  • No need to use h. when referencing new variables via the block form.
  • Inline form supports hash-like definition where names and values come in pairs and aren't separated: `one="One" two="Two";
  • Inline form can appear immediately next to the place variables are first used, improving code structuring for readability and maintainability.

I personally have doubts.

The block form separates names from values, making it hard to read and edit. Consider this synthetic example:

{{#with
  (some-math a b c)
  (some-math b a c)
  (some-other-math b d)
  (some-math c a b)
  (some-other-math d d)
  (some-other-math d a b)
  as |topLeft bottomLeft bottomRight topRight center focus|
}}

And now you have to update topRight. You'll have to count names then values to avoid a mistake. I find the {{#with (hash trick to be more practical when there are more than two variables.

As for the inline form, it turns purely declarative, functional, Lisp-like templates into imperative ones. When you see an unfamiliar variable in your template, it's no longer enough to look up the hierarchy for a with/let block wrapper. You also have to look for instances of inline let through the sibling code on the current level of hierarchy and on all parent levels, above and maybe even below the variable invocation! 😫 That's kinda against the nature of Handlebars.

Ideally, the compiler should be smart enough to merge multiple inline lets on the same level. But that would lead to overwriting conflicting variable. You append an let to your code, it shadows a variable from an earlier let invocation and breaks the code above that relies on the the old value. 😖 To avoid this, the compiler could refuse merging and instead demand only one inline let per level. But that would defeat the benefit of let appearing near first invocation of the defined variables.

Sometimes restrictions are good, and Handlebars are restrictive on purpose. Maybe we should rather upgrade the with helper to understand the hash-like form instead of introducing a new helper that essentially does the same job in a tricky way.

@cowboyd
Copy link
Author

cowboyd commented Jan 17, 2017

@lolmaus When I say caveats plural, I mean [], false, null, undefined and ''all as things that will magically prevent a with block from rendering its value. It's just not a generic primitive for binding names sinces it's riddled with corner cases.

As for the inline form, it turns purely declarative, functional, Lisp-like templates into imperative ones.

I disagree. There are absolutely no side effects with either the block form or the inline form since the inline form is transformed to the block form via a macro (itself a pure function). let in OCaml looks very similar to JavaScript:

let x = 5, y = 2 in
x + y

But we don't say that OCaml is imperative because unlike JavaScript = does not stand for assignment, but instead is a declaration of equivalence. Inline form for handlebars would be like that because Handlebars, like OCaml is a declarative language.

To your point though, I think that it should not be just a warning, but a hard error (as it would be in OCaml) to redefine variables at the same level of scope.

@cowboyd
Copy link
Author

cowboyd commented Jan 17, 2017

@simonihmig I see your point as the inline form is probably the most radical departure from the way people normally think of scope in handlebars, and while there is precedent for this style in other non-lisp functional languages, I'm considering decoupling it from this rfc. That said, it does prevent a lot of rightward drift when you have many values.

@simonihmig
Copy link
Contributor

@cowboyd Yes, it's just the inline form I have some concerns with. Just let me clarify this a bit more..

I think it is important to have the user in mind. And when we want Ember to not loose ground on other frameworks, we should focus on new and novice users and improve the DX there. I guess everybody who reads this will have a fairly decent knowledge of Ember and will adapt to new things easily, even if done in a weird way (not saying this is weird). But that should not fool us to think that this will be true for everybody else...

So having introduced Ember to new teammates myself, I know some issues they had. For example some were tempted to do something like this:

{{#if this.isEnabled()}}
  <p>{{this.getMessage()}}</p>
{{/if}}

This is an extreme example, just made up to clarify the underlying problem. While you don't have to know the details of how things work under the hood, you have to have the right mental model of how things work in principle. And in this case they were thinking in an imperative way, that is they assume they control the flow of operations and are in charge. While they are not. This is especially true if they have some background in other languages like PHP that allow to be used as template languages itself and mix markup and code. An example similar to the one above would be possible there, as it would be actually executed imperatively.

But in case of Ember and Handlebars, the better mental model would obviously be a declarative one, that describes what the solution should look like and not *how * it should be done. And maybe that of some kind of inversion of control, where the framework controls the flow of operations, and calls you (lifecycle hooks, CP) when needed.

While the inline let is technically certainly not imperative, I fear it could lead to users building or keeping that imperative mental model in their mind, because as I have said earlier it could look as if it is an imperative statement that is executed in some order, and that changes some (global or scoped, even more confusion possible) state by being executed.

@lolmaus
Copy link

lolmaus commented Jan 17, 2017

I've started an alternative RFC proposal: Allow the with helper accept named arguments. Aimed to make defining (and maintaining) multiple variables easier while staying true to the Ember way and preserving the declarative nature of Handlebars.

@Turbo87
Copy link
Member

Turbo87 commented Jan 18, 2017

FWIW some comments from my side:

  • I don't like the inline let form at all because it's no longer clear where those variables can actually be used (do they work everywhere below? do they work above? just in the current AST parent?)

  • Since we have {{#with}} already, which is roughly similar to {{#let}} but with several disadvantages as pointed out by @cowboyd I'd be okay with essentially deprecating {{#with}} (if it isn't already) and replacing it with {{#let}}

  • I do agree though that most of the logic should actually not live in the template because it can't be tested as well as a computed property, and allowing things like {{#let}} will likely make people think that this is the right way to go because it's so easy

@locks
Copy link
Contributor

locks commented Jan 18, 2017

I don't like the inline let form at all because it's no longer clear where those variables can actually be used (do they work everywhere below? do they work above? just in the current AST parent?)

The binding would be available from the {{let}} downwards, for the duration of that scope.

Since we have {{#with}} already, which is roughly similar to {{#let}} but with several disadvantages as pointed out by @cowboyd I'd be okay with essentially deprecating {{#with}} (if it isn't already) and replacing it with {{#let}}

☠️ {{with}}

@kellyselden
Copy link
Member

I have a feeling code like this would start popping up if we allowed inline let

// variable declaration area
{{let foo1=(bar ....)}}
{{let foo2=(bar ....)}}
{{let foo3=(bar ....)}}
...

// now start html area
<div>
...

Which brings us into a quasi js world where our imports/consts are at top, then we start coding. More of a visual problem for me instead of any technical problem.

@locks locks mentioned this pull request Dec 21, 2017
@locks
Copy link
Contributor

locks commented Dec 21, 2017

Hi everyone! I published an RFC that focuses on the block form of the helper: #286.

From the great discussion in this RFC, I thought it would be useful to split it up so we can incrementally ship this feature, and go into more detail into a potential future RFC for the inline form.

@locks
Copy link
Contributor

locks commented Sep 18, 2018

Closing as #286 partially superseded this proposal.

@locks locks closed this Sep 18, 2018
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.

10 participants