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

{{fn}} Helper #470

Merged
merged 8 commits into from
Apr 12, 2019
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions text/0000-bind-helper.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
- Start Date: 2019-03-20
- Relevant Team(s): Ember.js, Learning
- RFC PR: https://github.com/emberjs/rfcs/pull/470
- Tracking: (leave this empty)

# Bind Helper

## Summary

This RFC introduces a new helper `bind` to allow clear argument and context scope binding for functions passed in templates.

## Motivation

The current `action` helper has a lot of implicit and confusing behavior that is different than the Octane and post Octane programming model.

To understand the complexity of `action` there are many complex behaviors including:

1. Argument partial application (currying)
2. `this` context binding
3. `send` checks for Component and Controllers

At build time the `action` helper currently is passed through an AST transform to explicitly bind the `this` to be deterministic at runtime. This is a private API where the outputted Glimmer is not a 1-1 to the template. Also, the `action` helper is confused and has overlap with the `action` modifier which has similar but slightly different behavior.

Instead of this confusing and overloaded behavior, a new `bind` helper would be introduced to do partial application and context binding (with no need for build time private APIs).

## Detailed design

The `bind` helper will take in a function and then the set of arguments that will be partially applied to the function.
As an optional named argument the user can pass in a `this` property that will set the `this` context this named argument would default to `undefined`.

The behavior of this helper will be simple and use JavaScript's `function.prototype.bind`.

Here are some examples of the `bind` helper and the equivalent JS:

### Simple Case On Argument Curry

```hbs
{{bind this.log 1}}
```

```js
this.log.bind(null, 1);
```

### Multiple Argument Partial Application

```hbs
{{bind this.add 1 2}}
```

```js
this.add.bind(null, 1, 2);
```

### Setting the `this` context

```hbs
{{{bind this.session.logout context=this.anotherSession}}}

Choose a reason for hiding this comment

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

I hope the tiple braces are by accident?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think context is a pretty overloaded word that doesn't really convey a lot of meaning. Why don't we use thisArg to make it less ambiguous and match the EMCA spec's naming?

Choose a reason for hiding this comment

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

+1, or just this?

```

```js
this.session.logout.bind(this.anotherSession)
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I got a bit confused by looking at these examples, as they make it look like you have to pass all expected arguments, rather than partially applying them. Wouldn't the correct JS equivalent of {{fn this.add 1 2}} look more like this?

return function(...args) {
  this.add.call(this, 1, 2, ...args);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah actually that’s totally correct, we will update them!

Copy link
Contributor

Choose a reason for hiding this comment

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

@pzuraq it looks like this change didn't make it into the final version of the RFC text?


### Setting the `this` context and arguments
Copy link
Contributor

Choose a reason for hiding this comment

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

imo this is a little confusing since its the opposite of what the JavaScript bind function argument order is (wrt the context)

Can we make use of the array helper instead? Similar to how Angle Bracket does positional params (https://guides.emberjs.com/release/reference/syntax-conversion-guide/#toc_params-array)

{{bind this.car.drive context=this.honda params=(array 1000 'mile')}}

Copy link
Member

Choose a reason for hiding this comment

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

Doing this would be making the use of this helper too verbose and a step backwards with respect to the action helper.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we debated about this a bit, it comes down to “naming is hard”. What we want is a partial application helper. We’ve discussed:

  • partial - used by aome other languages, but we can’t because it means something in hbs
  • apply - also used by other languages, but already used in JS, and means something very different
  • curry - could work, but then we have to explain the term. Also, may not be technically correct (currying produces a function that recieves exactly 1 argument, or something like that)
  • papply - easy to misinterpet if you don’t know what the term “partial application”
  • partial-apply - fairly verbose for a common task

bind is the least bad choice IMO, it’s like if you made a bind function with a slightly better API, but I also agree it’s not ideal and could be confusing. I think curry is the second best out of those options, and am open to suggestions!

Choose a reason for hiding this comment

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

but isnt is confusing considered that in 90% of the cases {{bind will not be used to bind context but @action will bind context?

Choose a reason for hiding this comment

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

I got confused by that too, @luxferresum. If @action will be the recommended way to bind context, I prefer a helper called curry, apply-args or something on that direction to partially apply arguments.


```hbs
{{bind this.car.drive 1000 'mile' context=this.honda}}
```

```js
this.car.logout.bind(this.honda, 1000, 'mile')
```

### Comparison to Action Helper/Modifier

```hbs
{{!-- Actions --}}
<button {{action "increment"}}>Click</button>
<button {{action this.increment}}>Click</button>
<button onclick={{action "increment"}}>Click</button>
<button onclick={{action this.increment}}>Click</button>
<button {{action (action "increment")}}>Click</button>
<button {{action (action this.increment)}}>Click</button>

<button onclick={{bind this.increment context=this}}>Click</button>
<button {{on "click" (bind this.increment context=this)}}>Click</button>
```

### With `mut`

```hbs
{{!-- Actions --}}
<button {{action (mut showModal) true}}>Click</button>
<button onclick={{action (mut showModal) true}}>Click</button>
<button {{action (action (mut showModal) true)}}>Click</button>

<button onclick={{bind (mut showModal) true}}>Click</button>
<button {{on "click" (bind (mut showModal) true)}}>Click</button>
```

Because `function.prototype.bind` always binds the `this` context the bind helper would require functions to be already bound or the `this` named argument would need to be explicitly set.

## How we teach this

For guides we would switch to recommending the `bind` helper to pass functions into components args and modifiers.
In guides we would no longer recommend using the `action` helper based on the reasons listed in motivations.

## Drawbacks

Since this recommended design sets the `this` context to `null` using `function.prototype.bind` this could be confusing to junior developers if the function was not already bound (using something like the `@action` decorator or an `@autobind` decorator, JS binding, arrow functions, or the `bind` helper earlier in the stack).

## Alternatives

One alternative would be to continue using the `action` helper despite confusion and overloading behavior.

Another alternative would be to make the `bind` helper arguments exactly match `function.prototype.bind` (requiring `this` to always be passed in as the first argument). Doing this should probably introduce an `unbound` helper to allow partial argument application without changing `this`.

A third alternative would be to make the `context=` behavior not bind the `this` context. The ability to do this would need some introspection of what the context is of the function call (essentially recreating the complexity of `action`).

## Unresolved questions

> Optional, but suggested for first drafts. What parts of the design are still
How does this feel with `mut` since we do not `value=` syntax (possibly an `extract` helper or some syntax to get args from events or nested objects).