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

Introduce basic logic HBS helpers #152

Closed
wants to merge 2 commits into from
Closed

Introduce basic logic HBS helpers #152

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 16, 2016

@rwjblue
Copy link
Member

rwjblue commented Jul 16, 2016

👍 on new apps having access to these helpers out of the box, I personally find these to be somewhat required for general app development.

In my opinion, all things that can be done outside of Ember itself should be. These helpers are implemented perfectly well in user space, and require no changes to Ember core. I'd prefer the second avenue, inclusion of the addon into new ember-cli apps by default.

@workmanw
Copy link
Contributor

workmanw commented Jul 16, 2016

@rwjblue I generally agree with the idea of having default addons, but I think the gap between that and this RFC is the guides and docs.

I'm trying to imagine what it's like for a new ember user who's just installed ember-cli and created their first project. I think pretty quickly you arrive at a place where you need to handle basic logic conditionals in your templates. Speaking for myself alone, it wouldn't be my inclination to go looking around for addons and their docs to figure out how to do this. I worry some people spin their wheels trying to figure out something that honestly feels like it would be a language/framework/platform fundamental.

On the other side of the coin, I certainly appreciate wanting to keep the core as lean as possible. And the flexible addon system makes that very easy.

I'm not sure how you reconcile it, but I do think there is a learning gap here. Perhaps in some cases it is appropriate for the ember docs to incorporate some details of default addons.

@rwjblue
Copy link
Member

rwjblue commented Jul 16, 2016

@workmanw - Ya, I think I agree, but remember that a number of addons are already being discussed on guides.emberjs.com. Things that are considered fundamental to the "Ember experience" (i.e. ember-cli-qunit, ember-inspector, and ember-data). I see this as no different...

@rwjblue
Copy link
Member

rwjblue commented Jul 16, 2016

I guess what I'm saying is that "part of the default Ember experience" and "part of https://github.com/emberjs/ember.js" are very different things. 😸

@workmanw
Copy link
Contributor

I guess what I'm saying is that "part of the default Ember experience" and "part of https://github.com/emberjs/ember.js" are very different things. 😸

Yea I definitely agree with that.

@webark
Copy link

webark commented Jul 16, 2016

even though I feel it's clear that the community as a whole has spoken about this, and probably should be included by default, the reason for keeping them out has never been about keeping the core lean. handlebars refers to itself as "logicless" and there are definite benefits to that. Having come from the rails and php worlds, open templates quickly become an ugly monstosity with weird variable declarations, and huge conditional chains that just have no part in that space. one of the functions that a framework can perform, is steer the developer towards ideal patterns and paradigms, and with templates, and keeping them sane, a line needs to be drawn somewhere.

Having said that, given its popularity, I can understand including it by default, and if they where, would probably use them in the future. I wanted to however respond to @workmanw and offer a different perspective on why it's not included.

@ghost
Copy link
Author

ghost commented Jul 18, 2016

@rwjblue I'm 👍 to do the alternative.

@webark Ever since we've been able to use {{if}}, {{unless}} and even {{each}} or {{with}} handlebars (in Ember.js) has not been logicless. Allowing just these few logic helpers will not in any way allow developers to create templates that are an ugly monstrosity with weird variable declarations. It will just give your a little bit of extra power in your if / else logic.

Like in any piece of code, if you are overdoing it, then you should refactor it into something more simple and readable, this does not only apply solely to handlebars.

@webark
Copy link

webark commented Jul 18, 2016

totally..!! just meant to give more of a context as to why it's not in handlebars by default. Didn't want to imply that using these makes your code an ugly monstrosity.. just that the line was drawn at a conservative place, and a reason why that place was chosen.

@nathanhammond
Copy link
Member

We have been working very hard on the performance of Ember core, one of the things worth noting are these graphs: http://iao.fi/ember-size/

We've recently begun tracking canary and beta sizes as well to keep better tabs. More and more things should be moved to addons and should be. Especially in preparation for a future where we can automatically streamline your build.

@chancancode
Copy link
Member

FWIW...

These helpers are implemented perfectly well in user space, and require no changes to Ember core.

Presumably, if you want and and or, you probably want them to "short-circuit" – in (and false my-helper) or (or true my-helper), you probably won't expect my-helper to be called.

This is easy to implement in core but not currently possible in user-space. I don't think we will open up that possibility in the near future, so until then you cannot implement those boolean logic helpers purely using public APIs.

A more realistic example is something like (or cachedValue somethingExpensive).

@locks
Copy link
Contributor

locks commented Jul 21, 2016

@webark unlike Mustache, Handlebars never referred to itself as logicless, and @wycats can confirm/correct this statement. The question isn't as much about not having logic in templates, but what kind of logic should there be.

I for one install ember-truth-helpers first thing after generation a new project.

@chancancode raises an important question. Is the implication that we should implement and/or in ember.js itself and then addons can build on that?

@webark
Copy link

webark commented Jul 24, 2016

@locks again.. I'm not apposed to the idea. In fact, due to the overwhelming popular support, I'd say it's a definite 👍 Was purely trying to give some more perspective. And Handlebars does refer to it's self as logicless (from the github repo "Handlebars.js and Mustache are both logicless templating languages"), and here is a comment about handebars logiclessness (is that a word?). handlebars-lang/handlebars.js#1119 (comment)

Again 👍 .. didn't want to stir up anything .. Seems like it's a good idea to include it by default in ember-cli. ✌️

@wycats
Copy link
Member

wycats commented Jan 4, 2018

@martndemus are you still interested in this RFC?

@ghost
Copy link
Author

ghost commented Jan 4, 2018

@wycats thanks for reminding me. Actually yes I’d like that! I’d change my opinion to having truth helpers be a default addon, except when the short-circuit form would be implemented.

@wycats
Copy link
Member

wycats commented Jan 4, 2018

@martndemus I'd be in favor of the short-circuit form, personally.

A few comments:

  • I think you should document the semantics of these helpers in the RFC, rather than defer to the README of the truth helpers library.
  • I'm thinking that the numeric comparisons make sense as builtins. Can you add them?

@locks
Copy link
Contributor

locks commented Jan 25, 2019

I believe this has been superseded by #388. Thanks for the initial push Marten. Soon. Soon!

@locks locks closed this Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants