-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
{{on}}
Modifier
#471
Merged
Merged
{{on}}
Modifier
#471
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,216 @@ | ||
- Start Date: 2019-03-21 | ||
- Relevant Team(s): Ember.js, Learning | ||
- RFC PR: (after opening the RFC PR, update this with a link to it and update the file name) | ||
- Tracking: (leave this empty) | ||
|
||
# `{{on}}` Modifier | ||
|
||
## Summary | ||
|
||
Add an `{{on}}` modifier for adding event listeners to elements. | ||
|
||
## Motivation | ||
|
||
Currently, there are two ways to bind event listeners to elements in Ember | ||
templates with built-in, official Ember APIs: | ||
|
||
1. Use the `{{action}}` element modifier | ||
2. Use the `on*=` property bindings | ||
|
||
Both of these solutions are problematic for a number of reasons. | ||
|
||
The `{{action}}` modifier: | ||
|
||
- Uses a non-standard AST transform to pass `this` in order to bind the context | ||
of the template (the component) to the function (typically a component | ||
method). This is problematic behavior, not just because of the transform, but | ||
because of the implicit state that is being passed to the modifier in the | ||
first place, and the fact that the modifier is "magical" in the sense that it | ||
can do things that no other modifier or helper can do. | ||
- Binds to the `click` event by default, which requires a bit of knowledge to | ||
understand on first glance, and requires a bit of extra knowledge to figure | ||
out how to bind to other events. Compared to `onclick=`, this is less | ||
explicit, and requires more knowledge of the framework out of the box. | ||
- Is confusing when used with the `@action` decorator to bind the function, and | ||
the `{{action}}` helper. The term "action" is highly overloaded here, and it | ||
is unclear what it concretely means without context. | ||
|
||
The `on*` properties, by contrast, may seem to be a better approach overall to | ||
the problem, but they have their own issues: | ||
|
||
- The require the ability to bind to element _properties_. Properties are _not_ | ||
attributes, and the distinction is subtle, but important. Most importantly, | ||
props are _not_ rendered, and thus are _not_ SSR/rehydration friendly, since | ||
it is difficult to assign during rehydration. By constrast, modifiers are | ||
specifically designed around SSR. They do not trigger at all during server | ||
side render, and then are run specifically during rehydration. | ||
- Not all events are bindable via `on*` properties, and some events such as | ||
`focus` have different behavior than when assigned with `addEventListener` | ||
directly. This behavior has been a pain point in other frontend frameworks, | ||
and requires workarounds in most cases. | ||
- They do not work on certain elements at all, such as `<svg>`. | ||
- They are not compatible at all with standard web components. Web components | ||
don't send events in the same way as standard elements for properties, and | ||
instead rely on event listeners added via `addEventListener`. While web | ||
components are not currently used in the Ember ecosystem, they are advancing | ||
and may eventually be useful for smaller, self-contained components, | ||
especially for users that want to share code between framework ecosystems. | ||
|
||
Neither of these solutions is perfect for solving the problem of adding event | ||
listeners. This RFC proposes adding a new modifier to Ember: the `{{on}}` | ||
modifier. This modifier will explicitly add event listeners using the | ||
`addEventListener` API. | ||
|
||
## Credits | ||
|
||
The design of this modifier is based on [ember-on-modifier](https://github.com/buschtoens/ember-on-modifier) | ||
by Jan Buschtöns, which is an excellent addon that has allowed us to test this | ||
design in real apps and get feedback about the design. | ||
|
||
## Detailed design | ||
|
||
The `{{on}}` modifier will recieve: | ||
|
||
1. The event name as a string as the first positional parameter | ||
2. The event listener function as the second positional parameter | ||
3. Named parameters as options | ||
|
||
The following usages are equivalent: | ||
|
||
```hbs | ||
<div {{on "click" this.handleClick passive=true}}></div> | ||
``` | ||
|
||
```js | ||
element.addEventListener('click', this.handleClick, { passive: true }); | ||
``` | ||
|
||
Multiple event listeners can be added to an element by adding the modifier more | ||
than once: | ||
|
||
```hbs | ||
<div | ||
{{on "click" this.handleClick}} | ||
{{on "mouseenter" this.handleMouseEnter}} | ||
> | ||
</div> | ||
``` | ||
|
||
Extra positional parameters will be ignored. In order to pass values to a | ||
function, users should use a helper such as the [fn helper](https://github.com/emberjs/rfcs/pull/470): | ||
|
||
```hbs | ||
<div {{on "click" (fn this.addNumber 123)}}></div> | ||
``` | ||
|
||
The event will also be passed as a parameter to the function, so it can be used | ||
directly. | ||
|
||
`{{on}}` does _not_ bind context. Context binding must be handled via a helper, | ||
or in the component definition via the `@action` decorator, which will be the | ||
recommended path: | ||
|
||
```js | ||
class ClickComponent extends Component { | ||
@action | ||
handleClick() { | ||
// ... | ||
} | ||
} | ||
``` | ||
|
||
### Named Parameters | ||
|
||
The following named parameters will be accepted as options to `{{on}}`: | ||
|
||
- `capture` | ||
- `once` | ||
- `passive` | ||
|
||
These will be passed forward to `addEventListener` as options in modern | ||
browsers, and will be polyfilled in older browsers. Other options will be | ||
ignored. | ||
|
||
## How we teach this | ||
|
||
`{{on}}` itself can be taught in conjuction with event listeners. We can give | ||
users an overview of event listeners and the `addEventListener` API, and then | ||
relate the modifier directly to it. We should provide lots of examples of usage | ||
throughout the guides for how users can use it, including some examples of how | ||
to apply multiple event listeners to an element. | ||
|
||
`{{on}}` also does not bind context, and we should make it clear that the | ||
`@action` decorator is the recommended way to bind methods to their components. | ||
|
||
Other than that, most examples in the guides can be translated directly from | ||
the `{{action}}` modifier to the `{{on}}` modifier. | ||
Documentation work would include: | ||
|
||
- revising [Handling Events](https://guides.emberjs.com/release/components/handling-events/#toc_event-names) or the corresponding Octane article in entirety | ||
- revising [Triggering Changes with Actions](https://guides.emberjs.com/release/components/triggering-changes-with-actions/) or the corresponding Octane article | ||
- Updating 50+ uses of `{{action}}` in the guides | ||
- Adding `{{on}}` as a new API docs entry | ||
- Update examples within the API docs to show all uses of actions that are supported in the API docs - `{{on}}`, `{{action}}`, `onclick={{action}}` | ||
|
||
`{{action}}` is used 130+ times in the API docs. Not all examples would need to be updated, however an audit would be needed to figure out which need to be added to/changed. | ||
|
||
## Drawbacks | ||
|
||
- This API is somewhat more verbose than the `{{action}}` modifier since it | ||
requires a helper for passing additional values to the function. However, this | ||
explicitness makes it much more clear what the separation of concerns is, and | ||
allows users to fine tune their event handling. | ||
- The `{{on}}` modifier requires another helper to pass values. Currently, the | ||
ideal companion helper is the `{{fn}}` helper, which is still in RFC. | ||
- The `onclick=` style at first looks like a native browser API, and is | ||
sometimes easier to teach because of this. However, it is _not_ the same as | ||
the `onclick` attribute, which only receives strings, and the differences are | ||
often confusing. In addition, the fact that there are many use cases that | ||
aren't covered by these properties, and the fact that they are hostile to SSR, | ||
makes them less than ideal. | ||
|
||
## Alternatives | ||
|
||
One option would be to allow the `{{on}}` modifier to receive event listeners | ||
as named parameters instead: | ||
|
||
```hbs | ||
<div | ||
{{on | ||
click=this.handleClick | ||
mouseenter=this.handleMouseEnter | ||
}} | ||
> | ||
</div> | ||
``` | ||
|
||
One downside of this style of API is that there is no clear way to pass | ||
additional options to the individual event listeners. We could pass them as a | ||
positional parameter, but they would apply to _all_ listeners, which would | ||
require multiple invocations. | ||
|
||
We could also allow the `{{on}}` modifier to bind context. This opens up a | ||
question: Should helpers and modifiers by able to use the context of the | ||
template implicitly? Since helpers are analogous to functions in JavaScript, | ||
this would be an equivalent API choice in JS: | ||
|
||
```js | ||
function foo() { | ||
this.bar = 123; | ||
} | ||
|
||
class Foo { | ||
doSomething() { | ||
foo(); | ||
} | ||
} | ||
|
||
let foo = new Foo(); | ||
|
||
foo.doSomething(); | ||
console.log(foo.bar); // 123 | ||
``` | ||
|
||
Arbitrary functions being able to access `this` when used within a class like so | ||
seems problematic from a language design standpoint, and we believe an API like | ||
this in Ember's templates would be very problematic, and easy to misuse. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is kinda late and I apologize for that, but what will be the behavior, if the event name or event listener are missing or have the wrong type?
ember-on-modifier
currently checks fortypeof eventName === 'string' && typeof callback === 'function'
and only then registers the listener. If the check fails, no error will be raised. We might want to make this a hard assertion instead.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.
Agreed, I believe that we will validate name and listener via assertions. I suppose we could do something to allow null/undefined but it really seems more likely to be a bug than intentional.
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.
@richard-viney One solution would be using
(optional)
fromember-composable-helpers
, which returns a no-op function, if the input is not a function:While this requires extra gymnastics, I agree with @pzuraq and @rwjblue, that it is probably safer to assert against missing
eventName
andcallback
to prevent accidental errors.