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

Contextual Helpers and Modifiers (a.k.a. "first-class helpers/modifiers") #432

Merged
merged 24 commits into from Feb 5, 2019

Conversation

Projects
None yet
@chancancode
Copy link
Member

chancancode commented Jan 14, 2019

Rendered

chancancode and others added some commits Dec 20, 2018

@lifeart

This comment has been minimized.

Copy link

lifeart commented Jan 14, 2019

Is it possible to render curried helper with component helper?
Like

{{#let (helper "my-helper" 1 2 3) as |someCurriedHelper|}}
   {{component someCurriedHelper}}
{{/let}}

chancancode added some commits Jan 14, 2019

@wycats

This comment has been minimized.

Copy link
Member

wycats commented Jan 14, 2019

Another alternative is to keep the global namespace separate from the local namespace, thus avoiding the need for most deprecations. In practice, we believe this would result in much more confusion when things do not behave the way you would expect, but only in some niche corner cases.

If we went with this alternative, this problem would be amplified if template imports are introduced. In particular, transitioning from global access to component to an import could change the behavior. In my opinion, we should strive to avoid this kind of discontinuity when migrating from global access to imports (especially in templates that don't produce any deprecations).

@chancancode

This comment has been minimized.

Copy link
Member Author

chancancode commented Jan 14, 2019

Is it possible to render curried helper with component helper?

@lifeart it's syntactically valid, but line 2 will result in a runtime error (the value you passed in is not a component, similar to {{component 123456}}). {{helper someCurriedHelper}} will work (though unnecessary).

@lifeart

This comment has been minimized.

Copy link

lifeart commented Jan 14, 2019

@chancancode any way get curried instance type helper/component without exact knowledge of it?

for example:

// extenral scope

{{my-component 
  item=(random-left-or-right 
    (component 'foo-bar') 
    (helper 'zoo-buzz')
  )
}}

// my-component

{{item}}
// <- how I can get Item's type?
@drogus

This comment has been minimized.

Copy link

drogus commented Jan 14, 2019

I just wanted to say that it would be great to have it. I worked on a code that could be simplified by passing helpers. Here's one example: travis-ci/travis-web@b0f9cb2#diff-94481a0922ee4e3d9abcdefa49b300c2R73. I ended up exporting a function from a component, but since I couldn't pass it further down I used a compute helper, which is not ideal.

@chancancode

This comment has been minimized.

Copy link
Member Author

chancancode commented Jan 14, 2019

@lifeart If it's just between components and helpers, and only in the content position, you can just do {{item}} and it would work fine (the component and helper helper is not necessary for invoking something, even though it works). In general though, since they do pretty different things, it's probably pretty rare to pass them interchangeably anyway.

@lifeart

This comment has been minimized.

Copy link

lifeart commented Jan 15, 2019

@chancancode what if I need to decide should I use block statement for item or not?
( I have real-live example with ember-ast-hot-load, in templates parsing time it's impossible to be 100% sure is {{foo-bar}} helper or component, and now I do some booted-app lookup magic, but it will be nice to have an instrument what can return type of item)

@jessica-jordan jessica-jordan referenced this pull request Jan 15, 2019

Merged

Ember Times - Issue No. 81 - Jan 18th 2019 #3767

5 of 10 tasks complete
@samselikoff

This comment has been minimized.

Copy link

samselikoff commented Jan 15, 2019

wow. Super exciting 👌

@knownasilya knownasilya referenced this pull request Jan 15, 2019

Closed

Contextual helpers #208

@cibernox

This comment has been minimized.

Copy link
Contributor

cibernox commented Jan 15, 2019

I take that this would replace #208 I opened long ago. For what I've read the helper part very similar except that this doesn't require an invoke-helper keyword to call it.

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented Jan 16, 2019

@cibernox - Yes, I agree that this supersedes your earlier RFC.

@chancancode - Is there a way to call that out as "prior art" / thought in the space?

@chancancode

This comment has been minimized.

Copy link
Member Author

chancancode commented Jan 16, 2019

@cibernox thanks for working on this, I added your RFC to the alternatives section. Can you confirm this addressed the use cases you were designing for? Do you spot any further problems with the ambiguity in this new design?

@lolmaus

This comment has been minimized.

Copy link

lolmaus commented Jan 16, 2019

Please update the link in the top post after renaming. 🙇

@rwjblue
Copy link
Member

rwjblue left a comment

Overall I'm very much 👍 on this, thank you for taking the time to put this together!

I'd love to see a bit more discussion on a few things:

  • An overall transition guide. Do any of the propose changes have order of operations concerns?
  • A more precise (read non-prosey 😝) list of the deprecations being proposed here. This likely would include a rough transition plan for each as well as a bit of explanation around each deprecation being independent or not of other changes.
  • I think the "How do we teach" this section needs to include addition to the guides's template section (along with API docs).
Show resolved Hide resolved text/0432-contextual-helpers.md Outdated
produce an opaque, internal "helper definition" or "modifier definition"
object that can be passed around and invoked elsewhere.

* Any additional positional and/or named arguments will be stored ("curried")

This comment has been minimized.

@rwjblue

rwjblue Jan 16, 2019

Member

FWIW, I know that we generally call these positional arguments and named arguments today (as of "Glimmer 2"), but I don't think those terms are as well understood across the community.

In my experience, most folks call these params and hash. It might be nice to have a glossary/terminology section or something to clarify...

Show resolved Hide resolved text/0432-contextual-helpers.md Outdated
In the mean time, globals can be referenced explicitly using the `component`,
`helper`, and `modifier` helpers.

Another difference is how global helpers can be invoked without arguments in

This comment has been minimized.

@rwjblue

rwjblue Jan 16, 2019

Member

I think it's important to call out that this isn't about named arguments generally it's about named arguments to angle bracket invocations.

Specifically, this does not invoke pi (it passes the value):

{{my-component value=pi}}
positions (which includes argument positions for curly invocations), the
parentheses are already mandatory (otherwise it invokes the property fallback).

We propose to deprecate invoking global helpers in named argument positions and

This comment has been minimized.

@rwjblue

rwjblue Jan 16, 2019

Member

Is is this deprecation part of this RFC? Does it need its own transition path (ala https://emberjs.github.io/rfcs/0308-deprecate-property-lookup-fallback.html#transition-path)?

This comment has been minimized.

@chancancode

chancancode Jan 17, 2019

Author Member

It is part of this RFC. It should say "deprecate auto-invoking global helpers with no arguments in named arguments positions" and the transition path is from @foo={{bar}} to @foo={{(bar)}}.

Show resolved Hide resolved text/0432-contextual-helpers.md Outdated
Show resolved Hide resolved text/0432-contextual-helpers.md Outdated
Show resolved Hide resolved text/0432-contextual-helpers.md Outdated

Some additional details:

* When the first argument passed to the `helper` or `modifier` helper is

This comment has been minimized.

@cibernox

cibernox Jan 16, 2019

Contributor

Could you elaborate why you prefer this option over raising an error?

I'm assuming you did this thinking on {{helper @boundValue}}, but I'd say that in a non-bound form it should raise in build-time (p.e {{helper ""}}

This comment has been minimized.

@chancancode

chancancode Jan 17, 2019

Author Member

This matches the behavior of the component helper. In general, Handlebars tries to be quite forgiving about these things, and errors during render is generally unexpected (and currently unrecoverable).

It also enables use cases like:

{{#if this.featureIsEnabled}}
  {{yield (component "experimental")}}
{{else}}
  {{yield null}}
{{/if}}

..which the other side can further curry or "invoke" it without caring for whether the value is falsy.

I suppose we can statically disallow the {{helper ""}} but I don't quite see the point of it since 1) we have to support the runtime empty string anyway (see above) and 2) this involves using Magic™ that is otherwise not possible in user land (the compiler needs to know that the helper helper is special and disallow the static empty string).

I don't think it is worth the trouble.

Update text/0432-contextual-helpers.md
Co-Authored-By: chancancode <godfreykfc@gmail.com>

@jessica-jordan jessica-jordan referenced this pull request Jan 18, 2019

Merged

Ember Times No. 82 - Jan 25th 2019 #3773

9 of 13 tasks complete
problem. Alternatively, we could exclude the angle bracket invocation position
from being able to "see" implicit global identifiers.

### Local helpers and modifiers

This comment has been minimized.

@rwjblue

rwjblue Jan 24, 2019

Member

Based on the design, it seems like this is not limited to helpers and modifiers. Specifically if the design of this RFC is that these things are "just values" (which I think is roughly what is being said) then this section also applies to components.

avoid the conflict. With proper linting, this could be quite easily avoided
altogether.

With _implicit_ global bindings, this problem is might more difficult to spot

This comment has been minimized.

@knownasilya

knownasilya Jan 24, 2019

Contributor

'is might' typo

This comment has been minimized.

@chancancode

chancancode Jan 24, 2019

Author Member

much 😆

chancancode added some commits Jan 24, 2019

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented Jan 25, 2019

We discussed this at today's Ember.js core team meeting, and we believe that this RFC is ready to move into final comment period.

@luxferresum

This comment has been minimized.

Copy link

luxferresum commented Jan 28, 2019

Is it really a good idea that this is not equivalent?

<div class={{helper foo-bar separator=","}}>...</div>
<MyComponent @value={{helper foo-bar separator=","}} />

What will happen for

<MyComponent class={{helper foo-bar separator=","}} />

Should it invoke the helper or not? I honestly don't know the answer.
Does it not invoke it for angle bracket components or for @-arguments?

We just added so much clarity to the templates, I feel like we're removing clarity now!

Could we maybe rename helper to something like pass-helper and only allow it for passing a helper? Best would IMHO be a custom way to invoke the helper, but we already have what we have...

I really want to have this functionality, but do we really have no better ideas for the syntax?

@chancancode

This comment has been minimized.

Copy link
Member Author

chancancode commented Jan 28, 2019

@luxferresum in both cases you are “passing” the helper, it’s just what the position decide to do with the passed value that is different. In the named argument position, it passes the value to the component. In the attribute (and content) position, it sees that you are trying to “render” a helper value, and the only sensible thing to do at that point is to invoke it and then render the result. Alternatively, it could render [object Object] or maybe throw an error, but is that really helpful?

@luxferresum

This comment has been minimized.

Copy link

luxferresum commented Jan 28, 2019

Alternatively, it could render [object Object] or maybe throw an error, but is that really helpful?

I agree its not helpful, but maybe more expected.
Essentially it means that @foo={{helper my-helper}} is passing a helper instance while foo={{helper my-helper}} is invoking it. While reasonable it still is hard to explain I think!

Don't get me wrong, I don't really have a better idea. But at least I would disallow using {{helper when not passing the helper. Probably it would be best to just throw an error for class={{helper foo-bar separator=","}}.

What feels like a better solution would be a sigil to invoke helpers. But this would break what we already have. Something like class={{!my-helper}}. The same way we use () ins JS to invoke a function. (foo=bar vs foo=bar())

@wycats

This comment has been minimized.

Copy link
Member

wycats commented Jan 29, 2019

@luxferresum thanks for all the detailed feedback!

Is it really a good idea that this is not equivalent?

<div class={{helper foo-bar separator=","}}>...</div>
<MyComponent @value={{helper foo-bar separator=","}} />

What will happen for

<MyComponent class={{helper foo-bar separator=","}} />

Is it really a good idea that this is not equivalent?

<div class={{helper foo-bar separator=","}}>...</div>
<MyComponent @value={{helper foo-bar separator=","}} />

What will happen for

<MyComponent class={{helper foo-bar separator=","}} />

A few things.

First of all, in all three cases, helper foo-bar separator="," is creating a new helper instance, ready to be invoked.

When you pass it as a named argument (as @value={{helper foo-bar separator=","}} or value=(helper foo-bar separator=","), it's passed directly, ready to be used by the other side.

When you pass it as an attribute (as class={{helper foo-bar separator=","}}), it's also passed as a value to Ember, which has a choice to make. It could treat the value as any other object and stick [object Object] into the attribute position, it could throw an error, or it could invoke the helper. Any of these are valid choices, but this RFC decided to invoke the helper.

Passing it as content (as <p>{{helper foo-bar separator=","}}</p>) is the same thing: it's passed as a value to Ember, and this RFC decided that invoking the helper is the best choice.

This distinction has nothing to do with angle bracket invocation vs. curly invocation. It has to do with the fact that Ember has decided to invoke the helper, while the user-space code (the component implementation) can choose between invoking the helper, stashing it off, or even adding more arguments before invoking it.


Also of note: it's very unlikely that this distinction will come up when using the helper helper directly: class={{helper anything}} is a silly thing to write, and we'd probably want to lint against it.

However, this forms is more likely:

{{#let (helper foo-bar separator=",") as |foo-bar|}}
  <div class={{foo-bar}}>hello world</div>

  <Tab @helper={{foo-bar}} />
{{/let}}

In this case, it's very important that foo-bar is passed directly into the Tab component, because the Tab component may want to stash it off or add more arguments. When passed as a class, Ember is the one that you're passing the value to, and there's no reason to avoid invoking it. In all cases, it starts off as a value and is invoked when the code you're passing it to decides to invoke it.

This will become more relevant with import syntax:

--- js ---
import { title, upcase } from "./title-helpers";
import { Title } from './Title';

--- hbs ---
<div class={{title}}>hello world</div>

<Title @transform={{upcase}} @title="Hello world" />

With this definition of title-helpers.js:

import Helper from '@ember/component/helper';
import { inject as service } from '@ember/service';

export const title = Helper.extend({
  title: service('title'),

  compute() {
    return this.title;
  }
});

export const upcase = helper(([string]) => string.toUpperCase());

And this implementation of Title.js:

<h1>{{@transform @title}}</h1>

In this case, it's important that our component can pass the uninvoked upcase directly through for invocation by Title.js (to allow Title.js to put together the pieces). This particular example is trivial, but passing along a function to invoke with arguments as desired by the component is a powerful capability that exists in JS and this proposal aims to support.

The distinction here is that the <Title> call is passing the value to more user-space code, which needs the flexibility to decide what to do with it. Ember, on the other hand, is the end of the road, and has nothing to do with the helper other than invoke it.


Finally, note that in the previous example, I needed to use a class-based helper to illustrate a situation with class={{title}}. This is because it's very, very rare to have a situation where you have a helper that can be invoked with zero arguments and is meaningful.

For this reason, we expect this confusion to be less serious than it first seems. The confusion would come from noticing that you don't need () in attribute position, and then forgetting to use them in argument position.

First, zero-argument helpers are simply rare. The most likely reason to write @arg={{someHelper}} is not that you're trying to invoke someHelper, but rather than you're trying to pass it. Similarly, you are very unlikely to encounter zero-argument helpers in attribute position at all.

Second, we would have a development-mode assertion when attempting to use a passed-in helper as a value (using the same proxy-wrapping strategy we use elsewhere in the framework). The solution (as the assertion will say) is to say @arg={{(someHelper)}}. This is a little bit noisy, but in light of the expected rareness of zero-argument helpers, it seems that the explicitness adds more value than the alternatives.

@wycats wycats closed this Jan 29, 2019

@wycats wycats reopened this Jan 29, 2019

@jenweber

This comment has been minimized.

Copy link
Contributor

jenweber commented Feb 1, 2019

@chancancode can you recommend a specific section of the Guides' existing content that should be used to teach this? Also a recommendation of what level of detail belongs in the guides, versus that appropriate to just have available in the API docs. Thanks!

@chancancode

This comment has been minimized.

Copy link
Member Author

chancancode commented Feb 1, 2019

@jenweber They should definitely be available in the API docs first and foremost. For guides, my recommendation is the "How we teach this" section. In terms of the current guides, do we already teach the component helper? If not, I think it is okay to keep it API docs-only for now, while we work on restructuring the guides to implement the recommendation in the RFC.

@rwjblue rwjblue merged commit 6f6663f into master Feb 5, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rwjblue rwjblue deleted the contextual-helpers branch Feb 5, 2019

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented Feb 5, 2019

🎉 - Sorry for the delay here, I was supposed to land this on Friday....

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