It would be nice to have support for the EventListener interface.
So it wouldn´t be necessary to bind
the listeners, creating unnecessary copies of the callback for each instance of a component, making use of Event Delegation.
Does it make any sense?
I´ve made a working proof of concept in this plunk
This is my first approach (tests will be added)
@sminutoli Do you have any benchmarks that indicate that creating such an object would be any faster than binding a function?
It's not clear to me what problem this is solving or how the solution is related to the problem I assume you're trying to solve. You mention binding but you would still need to bind if you want to call instance methods. Maybe you're referring to paying the binding cost even if the function isn't called? In that case I think I'd like to redirect to #6908 where there's a bit of discussion.
We already make use of event delegation (we attach single listeners for each event at the top level) so I think that might be tangential.
Obviously I'm not understanding clearly so if please correct me if I've made incorrect assumptions :)
Hi @zpao, first of all thanks for spending some time trying to understand my proposal.
I´m not trying to solve any problem, just to support the actual DOM EventListener
interface as it works on modern browsers.
Using an object that implements the interface, you don´t need to bind if you want to call instance methods at all, because of the prototype delegation.
The bind thing is only one of various benefits. The EventListener
interface lets you to decide if you want to share one object to handle several events, reusing it in other contexts, and a lot of OOP creativity instead of a simple callback execution (un-invertion of control?). Another useful thing is that calling the object instead of a plain function, you can take advantage of dynamic updates to the implementation in a transparent way (If you play with the example provided in my plunk, you can notice that after changing the add
method, the binded function remains "old", because it´s a copy of the original function). Another one, If the handleEvent
was on the prototype
chain instead of the actual instance and you create 1000 instances, you can reuse the very same handle thanks to prototype delegation.
As a side note, I don´t see any problem adding this support, I´m not making a proposal in order to _change the actual behaviour_, using functions as callbacks its OK, but it would be nice to have the option of using an EventListener
object as well.
@jimfb I´m not a benchmark guy yet, but if you see libraries using good prototypical OOP techniques like stampit it does matter not to copy a function...
@gaearon related to #6908, maybe the solution for those that don´t want to use bind everywhere is to implement the EventListener interface
available in pure vanilla js DOM Event API.
I think that the actual onClick
implementation it´s OK and compatible with the DOM API when using callback functions. I agree with the react team that extracting the variable is odd. It seems like the native DOM EventListener interface
was created to solve this sort of things, using an object polymorphic with the handleEvent message...
The EventListener interface lets you to decide if you want to share one object to handle several events, reusing it in other contexts, and a lot of OOP creativity instead of a simple callback execution (un-invertion of control?).
If the same handler can be used to handle several events, you could just pass in the same function to all those locations, right? It's not like you need to re-bind every time.
I´m not trying to solve any problem, just to support the actual DOM EventListener interface as it works on modern browsers.
A question is: is there any REASON to support this API. Currently, people may accept an event handler, and wrap that handler in another handler that calls the original delegate as part of the new function's execution. This pattern becomes much more difficult if you always need to remember that the original/delegate MIGHT be an EventListener interface instead. So we would probably only want to do this if it provides tangable wins (like performance).
@jimfb for all devs that are _unaware_ of this native behaviour on vanillaJS there's no benefit nor disadvantage to support this feature. For devs who wants to use this feature its great.
My question is, why do I have to use the EventListener API
in a limited way on React? Why not to add the support to the full EventListener API
as stated in the specs?
I know this usage of EventListener interface
is something pretty unknown by the majority of js devs, but it doesnt seem to have any disadvantage to add the support. In my implementation, I've only changed a few lines and it works pretty well (obviusly it needs some review from the smart guys).
Maybe using the EventListener interface
can solve or lead to a successful pattern on dealing with this
and bind
and so on...
@sminutoli There is a disadvantage...
Currently, people may accept an event handler, and wrap that handler in another handler that calls the original delegate as part of the new function's execution. This pattern becomes much more difficult if you always need to remember that the original/delegate MIGHT be an EventListener interface instead.
If a dev is unaware of this feature, there IS a disadvantage, because their wrapper function will fail exceptionally when it tries to "invoke" the delegate EventListener
(which is now an object instead of a function).
I don't think it's worth the additional complexity at this point. As @jimfb mentioned, the change will require that any component that calls a parent handler will need to introspect the handler to see if it's an object or a function. Right now it's simple and consistent and I don't see enough gains by taking the approach you are recommending. I think it's interesting and I appreciate the discussion, it could influence the way we think about potential changes, but I don't think it's the right thing to do right now.
cc @sebmarkbage. I heard you were thinking about something related to this recently. Perhaps you have some final comment before we close this.
@zpao did you see how I roughly added the feature here? Maybe I'm missing something but it seems like simply adding the logic on the EventPluginHub
it works.
The _only_ acceptable value to the handler is a EventListener interface object
, any other attempt will throw (like now when you pass a non-function as a handler).
Thanks for taking this prop in consideration!
I've updated the plunker to clarify a little the purpose of the demo.
@jimfb I can't see your point. If I'm getting you right the "disadvantage" you're mentioning is indeed a bad implementation. Nowadays the DOM Event API
works exactly like I'm proposing and it's not a big deal. And it has benefits for those who know how to take advantage of delegation.
I did see the implementation. In and of itself it's not complex. But supporting it in all existing components is where it gets complex. Every component that is doing the below will need to add an additional type check and do 2 different things. This is a common pattern, where a component composes another but adds additional functionality to the event handler.
// My App renders this
<MyButton onClick={this.someFunction} />
MyButton = React.createClass({
propTypes: {
onClick: PropTypes.func.isRequired
},
handleClick(e) {
// do something first
this.props.onClick(e)
}
render() {,
return <input onClick={this.handleClick} />
}
})
@zpao I understand better now your concerns... Adding this feature will work on the basic JSX elements (those which really subscribe/handle the listeners). A wrapper component like your <MyButton>
is exposing a public API where onClick
must be a function (as is stated in the propTypes
).
If we supported the EventListener
interface, when you´re _writing_ a component like <MyButton>
you can _choose_ doing something like...
//at this point "onClick" would be "onAction" or "onWhatever", because ISNT the real onClick :)
<MyButton onClick={this.theHandler} />
class MyButton extends React.Component {
constructor(){
super();
}
handleEvent(evt){
//call to the callback required in the props
this.props.onClick(evt);
//or call to a method present in the prototype chain, runtime evaluated!
//indeed, your MyButton class could extend another proto with a handleEvent, and so on...
}
render(){
//no need of bind!
return <input onClick={this} />
}
}
No autobinding. The handleEvent
function is the same in the MyButton.prototype
.
Obviusly, you can _choose_ not to using at all the support, and use the common pattern you´ve exposed.
Thanks for keeping track on this.
If we were to do this, we'd probably provide a custom helper so that composite components can dispatch an event to either a function
or EventListener
.
function dispatchEvent(handler, event) {
if (typeof handler === 'function') {
handler(event);
} else {
handler.handleEvent(event);
}
}
We'd then encourage everyone to write components like this:
class MyButton extends React.Component {
handleEvent(event) {
if (event.type === 'click') {
dispatchEvent(this.props.onClick, {
type: 'click'
});
}
}
render() {
return <input onClick={this} />;
}
}
However, it seems to me that components tend to handle a lot of different types of event from different sources, that this pattern ends up being used very rarely and functions preferred. At least that seemed to be the case from my experience with this pattern in the past.
You end up writing a lot of additional forwarding boilerplate code that is basically just:
handleEvent(event) {
switch (event.type) {
case 'click':
return this.handleClick(event);
case 'mouseenter':
return this.handleEnter(event);
case 'mouseleave':
return this.handleLeave(event);
...
}
}
Therefore, it seems unnecessarily restrictive to force everyone building composites to correspond to this interface when it just simpler to say: Everything is a function and you can pass whatever to it.
It is also unclear whether the binding itself is actually a perf problem or if there are other issues - such as how it plays with shouldComponentUpdate
and/or the generational GC.
That said, there might be other valid reasons to require a dispatchEvent
pattern. For example, we can now schedule the callback to a later point if needed to ensure that its props are consistent and it is a good time to handle the event.
The handleEvent
pattern also has a nice symmetry with Flux/Redux patterns which makes it clearer that state changes are really just the same pattern with different root identities.
If we want to do this, we should first figure out what it means for functional components though.
Since they don't have a this
I suspect it might be tricky to have so many different ways of doing this.
I'll close this out for now but let's keep seeing if there are more related things that might make it worth having this pattern available.
cc @gaearon Regarding unifying event handlers and Redux patterns.
@sebmarkbage thanks a lot for your feedback.
I know that the handleEvent
pattern needs to be explored, i.e., you can use delegates
to achieve something similar to the switch
but in a OOP way, like...
class MyButton extends React.Component {
constructor(){
super();
this.delegateClick = Object.create(this);
this.delegateClick.handleEvent = this.handleClick;
this.delegateChange = Object.create(this);
this.delegateChange.handleEvent = this.handleChange;
}
handleClick(evt){
}
handleChange(evt){
}
}
render() {
return <input onClick={this.delegateClick} onChange={this.delegateChange} />;
}
}
You can share a delegate
across instances as well...
class MyButton extends React.Component {
constructor(){
super();
}
handleClick(evt){
}
handleChange(evt){
}
}
render() {
//this.delegateClick is found on [[Proto]].delegateClick
return <input onClick={this.delegateClick} onChange={this.delegateChange} />;
}
}
MyButton.prototype.delegateClick = Object.create(MyButton.prototype);
MyButton.prototype.delegateClick.handleEvent = function handleEvent(evt){
//delegates on MyButton.prototype
this.handleClick(evt)
};
MyButton.prototype.delegateChange = Object.create(MyButton.prototype);
MyButton.prototype.delegateChange.handleEvent = function handleEvent(evt){
//delegates on MyButton.prototype, runtime and dynamic
//if the instance overwrites handleChange, it will be affected
someCondition ? this.handleChange(evt) : this.handleChange2(evt)
};
Or you can be more creative, this is the OOP door that supporting the EventInteface
opens... And for those that don't care about this sort of things, they could just keep doing the basic _callback_ handler, there´s no need to adopt the handleEvent
pattern.
React.Component.prototype.handleEvent = (function () {
// all known React.Component events
var map = {
click: 'onClick',
change: 'onChange'
};
// fallback helper
function capitalize(type) {
return 'on' + type.charAt(0).toUpperCase() + type.slice(1);
}
// one handleEvent to rule them all
return function handleEvent(e) {
var method = map[e.type] || capitalize(e.type);
if (method in this.props) this.props[method](e);
};
}());
As mentioned by @sminutoli who knows the pattern passes this
and who doesn't use what's been used 'till today.
The trade off is a slightly slower dispatch VS extremely improved memory consumption where thousand components won't need _O(instances * boundMethods)_ extra references.
This can be benchmarked too. I've create a simple test here:
https://gist.github.com/WebReflection/5aaca727bc3b784d43be3704cf65abff
Check bound and Arrow actual memory consumption VS classes based on inherited handleEvent
.
No React involved here but ... hey, let developers decide, it looks like a win-win for everyone.
This is was a I was talking about, OOP creativity!
sure thing @sminutoli yet @sebmarkbage closed this a year ago.
Let's try again to convince them handleEvent
is easily benchmark-able the best way to go:
https://medium.com/@WebReflection/dom-handleevent-a-cross-platform-standard-since-year-2000-5bf17287fd38
Is anyone interested in working on a HOC implementation of this distributed as a library? This is an interesting pattern but there seems to be some option paralysis in actually implementing it, a convention over configuration library could be handy if I'm understanding this correctly.
@nickmccurdy, here is my humble attempt https://github.com/alexeyraspopov/react-handle-event.
What about the difference between onClick
and onCustomThing
? Either components would have to implement this same mechanism for custom callbacks somehow, or devs would always have to remember to bind only the custom callbacks.
Maybe React could set e.g. this._owner
to the component that created the current component so it could be used for calling.
Doesn't seem straightforward. Also, normally you don't create thousands of React elements at the same time…
Another application of this would be element serialization in, say, JSON. There is no way (without transpilers) to serialize closures or Function.prototype.bind variables.
While in case of an object with a function we could keep the functions in prototypes, and data it requires in the object. The number of prototypes is usually finite, so this would work for many applications. Well, it is still possible the function to get the data from the element though, but it is more complicated.
You're welcome to open an RFC for this.
https://github.com/reactjs/rfcs
I think it's plausible we'll want to support this at some point.
Most helpful comment
As mentioned by @sminutoli who knows the pattern passes
this
and who doesn't use what's been used 'till today.The trade off is a slightly slower dispatch VS extremely improved memory consumption where thousand components won't need _O(instances * boundMethods)_ extra references.
This can be benchmarked too. I've create a simple test here:
https://gist.github.com/WebReflection/5aaca727bc3b784d43be3704cf65abff
Check bound and Arrow actual memory consumption VS classes based on inherited
handleEvent
.No React involved here but ... hey, let developers decide, it looks like a win-win for everyone.