-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Helpers #53
Helpers #53
Conversation
|has positional params|has template|has lifecycle, instance|can control rerender | ||
---|---|---|---|--- | ||
components|Yes|Yes|Yes|Yes | ||
function helpers|Yes|No|No|No |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function helpers can yield to templates. if
for example is a function helper today...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
helpers can yield but do not have a template. Do you think it would be more accurate to say they do not have a layout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you were saying they cannot "accept a block" (aka the template) which is not correct. The terminology of layout
and template
are annoying, and I am sorry that you have to thread the needle here...
@mixonic you've made my day 😀 My two favorite points are making I'm definitely for |
|
||
* The helper instance is created. | ||
* The `compute` method is called, and its return value is treated like the | ||
return value of a function-based helper. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not obvious to me what exactly is done with the return value of a function-based helper. Can you potentially list a few in context examples for:
- sub-expr:
{{#each (get model recordType) as |record|}} {{/each}}
- element space:
<div {{get model recordType}}></div>
(I don't like this and it should be discouraged for sure) - attribute assignment:
<div data-name="{{get model recordType}}"></div>
- "normal" (aka none of the above):
<p>{{get model recordType}}</p>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a section on return values, and ran some tests. I discovered that function-based helpers don't work in some of the places I describe they should. I'd like to make them work in those cases.
This is going to be very popular with addon authors :) I don't like I like plain |
|
||
Porting this usage to a stateful helper only changes the boilerplate: | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
js block
Looks wonderful. A couple points:
|
The |
I do like this proposal better than the subexpresions one. This kind of helpers remains as close as possible to the existing helpers, that are the simplest abstraction you can have for getting an output from some input, just adding the minimum amount of functionality to let them I'd like the to be one way and not having access to the context. That would limit side efects changes to things you're injecting, and ideally you should keep that to a minimum. |
|
||
They fill a gap in Ember's current template APIs: | ||
|
||
| | has positional params | has layout | can yield template | has lifecycle, instance | can control rerender | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily feel like this table is adding a lot to understanding the problem, and it makes it look like each of the columns is a legitimate "primitive knob" that you might want to tune separately. I also think the comparison with Component makes it seem even more like there are knobs being tuned.
The way I'd talk about it is:
- Components are stateful and manage an area of DOM
- Helpers produce a single value that can be passed into other helpers, but don't directly control an area of DOM
Ember.Helper
s have a full lifecycle and access to services, and have the ability to trigger a recomputation
when something external changes.- There is a shorthand for
Ember.Helper
that you can use if you have no dependencies on external services.
Awesome start, @mixonic. I'd prefer to descope using stateful helpers in block form and focus solely on the subexpression case. Once we've seen how that shakes out, we can circle back around and better understand the use cases for the block form. Personally, I'd rather push people in the direction of components if they need block semantics, especially now that @ef4 has implemented positional params. The biggest downsides of Components today is that they cannot be used in subexpressions, which IMO should be the major focus of this RFC. Lastly, the block form tempts people to make a comparison of stateful helpers to Components. By eliminating the block form for now, you can dramatically simplify and clarify the narrative: Stateful helpers are just regular helpers that have access to services, and here's the syntax for building them. I agree with the above comments that |
# Motivation | ||
|
||
Helpers in Ember are pure functions. This make them easy to reason about, but | ||
also overly simplistic. It is difficult to write a helper that recomputes due to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would say this a little bit differently.
Historically, helpers in Ember have had access to a lot of ambient information in the scope. This made them powerful, but also error-prone. In practice, they were used by addon authors to implement powerful functionality.
Glimmer does not want to expose the guts of the scoping system to user code, so our initial stab at helpers was to make them pure functions: data in, data out. That works well for a lot of cases, and is very ergonomic, but is a regression for addons that need the value they emit to be based on external state. This proposal is an attempt to recoup enough expressiveness so that these addons can retool for Glimmer without exposing the guts of the scoping system to addons again.
Relatedly, we want to be able to expose an API that we can continue to maintain over the 2.x series, unlike the private "guts" APIs that have had to change multiple times in the 1.x series.
@wycats said,
That suggests that this RFC probably needs to consider the public API for that shorthand. ember-cli/ember-cli#3710 may be of interest. I opened that because the guides and ember-cli differ about how to build pure-functional helpers, and both recommend using APIs that are currently private (though widely assumed to be public). |
I've made a significant set of changes this morning:
|
@rlivsey "What's the difference between a stateful block helper and a tagless component?" This is exactly Tom's thought, and is why I've removed the block stuff. |
See #58 for a detailed proposal about dashless helpers. |
How would you deal with helpers that take in promises? |
With a (full) Helper, this is straightforward: just call @mixonic the current guides use Ember.Handlebars.helper('fullName', function(person) {
return person.get('firstName') + ' ' + person.get('lastName');
}, 'firstName', 'lastName'); I see two options for this in the new world:
|
@mixonic - Can you double check the final details on this (as compared to the implementation PR), rename the file (increment the number) so we can merge? |
@jamesarosen for now, I'd like to say you need to use the full helper syntax for additional bindings. We can improve this, just need to debate the sugar separate from shipping in 1.13 :-) |
session: Ember.inject.service(), | ||
onCurrentUserChange: Ember.observes('session.currentUser', function() { | ||
this.recompute(); | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a common occurrence, could it be achieved in a more declarative manner?
Maybe something like:
export default Ember.Helper.extend({
recomputeWhenTheseThingsChange: ['session.currentUser', 'session.isLoggedIn'],
});
EDIT: (Sorry, late to the party on this one. Please ignore if this has already been discussed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to see what cases are common before adding sugar, especially since ES7 decorators will shortly make many of these things much nicer to express.
Will this be in Ember 1.13 in the beta cycle or will it be in the Ember 2.0 (beta) cycle? |
@Eptis This is already in 1.13 (although probably a 1.13.beta3 should be released) |
@Eptis - It will be in 1.13 |
View as HTML
Pardon any spelling snafus etc, written on a 🚌 :-p