[RFC] Per React container event listening/dispatching #2050
Conversation
| */ | ||
| var topListenersIDKey = "_reactListenersID" + String(Math.random()).slice(2); | ||
| // TODO: (chenglou) should we hide this to prevent traversal problem? | ||
| var eventsKey = '_reactEvents'; |
There was a problem hiding this comment.
I'm curious why there is a need to store an object on the DOM node, why not use an internal object indexed by the React ID instead?
|
Didn't consider nested Edit: seems like it took care of itself in |
|
👍 This should be a major step towards playing nice with non-React DOM and separate React instances. |
|
Great to see this happening! |
Work in progress. `enterleave` plugin (and maybe `analyticsPlugin`) is broken because it relied on the old behavior. But wanted to put this out here for suggestions. There are also some comments that need to be changed, I'll do it when the code is finalized. If we attach the event listening/dispatching at container level, it'll benefit the case of `<Editor/><Plugin1/>` (both are container roots), since `Plugin1` won't disturb `Editor`. We also detach those listeners now. There wasn't really a need in the past. Fixes facebook#2043 Should help with facebook#1964
|
@nathansobo what else do you need for react atom to get faster? |
|
@nathansobo this is kind of a big change and it's not set in stone. And frankly I'm not even sure how much this helps. |
|
@chenglou It seems to me that it is a critical step towards playing nice with other frameworks and instances. |
|
@chenglou Avoiding event handling for the input field on a per component basis sounds great, but it's "playing nice with other frameworks" that we're interested in more than anything. Performance problems can always be hacked around with manual DOM manipulation if needed, but it would be great to implement individual elements with React without the need for a container. |
|
Whats the current status of attaching event listeners at react root containers/components instead of the document level ? I've got some issues with 3rd party code interfering with reacts synthetic event system - so their listener callback are triggered as well, although I've tried to use 'e.nativeEvent.stopPropagation()'. Since the review is still open, are there any plans to integrate it or at least provide an option to bind the scope on demand ? |
|
I think at this point we aren't going to do this. We can revisit later if we need. |
|
As @syranide and @autarc said is really difficult to integrate React in existing non React projects if all events are attached to document. We are integrating React in our platform but this issue is breaking expected event bubbling behaviour. It's impossible for us to stop the propagation for some events without hitting other bound libraries. Before arrive to this issue and understand the current status I built this example to know the impact: http://jsbin.com/rafitiq/edit?html,js,output Is there some workaround to make this work properly? |
|
I'm sure that I'm not aware about something that was already discussed but I created this PR and the previous example is working as expected. |
|
We attach events to roots in 17. |
Work in progress.
enterleaveplugin (and maybeanalyticsPlugin) is broken because it relied on the old behavior. But wanted to put this out here for suggestions. There are also some comments that need to be changed, I'll do it when the code is finalized.If we attach the event listening/dispatching at container level, it'll benefit the case of
<Editor/><Plugin1/>(both are container roots), sincePlugin1won't disturbEditor.We also detach those listeners now. There wasn't really a need in the past.
@spicyj @zpao @nathansobo
Fixes #2043
Should help with #1964