Respect event.stopPropagation() when nested

[lrowe]

When nesting top-level components (e.g., calling React.renderComponent within
componentDidMount), events bubble to the parent component.

This change ensures that when event.stopPropagation() is called in an event
handler of the inner top-level component, event handlers of outer top-level
components are not called. Closes #1691.

As of the current W3C DOM4 draft, there is no standardised way to determine if
stopPropagation() has been called on an event. This change sets the
deprecated cancelBubble property existing in most current browsers and
should continue to work if the property is removed in the future.

[syranide]

I think we explicitly switched away from always setting event.cancelBubble = true; because Chrome actively complains on it being deprecated.

[lrowe]

I don't see the deprecation warning from setting event.cancelBubble from this jsfiddle in current Chrome/Firefox/Safari: http://jsfiddle.net/lrowe/5EcSz/

It seems only Firefox will update event.cancelBubble on event.stopPropagation(): http://jsfiddle.net/lrowe/KvcSE/

An alternative would be to set a custom property (propagationStopped, _react_propagationStopped, ...) on the nativeEvent.

[syranide]

@lrowe Sorry, yeah you're right, I was thinking of returnValue (which emits the warning). Updating the nativeEvent seems like a bad idea, as React explicitly (I believe) tries to avoid affecting the environment, in any way. However, since we only support running a single instance of React at any time (I think?) it seems like modifying the nativeEvent shouldn't be necessary, although I don't think it's a good idea to use cancelBubble as it might give some the idea that it can also be manually set.

[lrowe]

@syranide avoiding any modification to the nativeEvent will require more significant changes to the current event code. The value of the synthetic event.isPropagationStopped() would need to be communicated from executeDispatch up to handleTopLevelImpl.

• ReactEventListener.js handleTopLevelImpl - For each top level react component in the dom hierarchy, with the nativeEvent:
  • ReactEventEmitterMixin.js ReactEventListener._handleTopLevel(..., nativeEvent)
    • EventPluginHub.extractEvents(..., nativeEvent) - maps single nativeEvent to multiple synthetic events.
    • EventPluginHub.enqueueEvents(events)
    • EventPluginHub.processEventQueue()
      • forEachAccumulated(processingEventQueue, executeDispatchesAndRelease)
        • executeDispatchesAndRelease
          • EventPluginUtils.executeDispatchesInOrder(event, executeDispatch)
            • EventPluginUtils.js executeDispatch - currently returns returnValue, but that is ignored by caller.

This also raises the question of what should happen within a single top level react component when there are multiple synthetic events for a nativeEvent. Should calling event.stopPropagation() on one of the synthetic events also stop propagation of the others?

[sebmarkbage]

It would be useful to have bubbling of events follow a different order than the DOM. E.g. Layers that are technically part of a component but gets rendered outside of the component. We should have more explicit and defined behavior for how you can nest renderComponent calls that render to different targets.

That's a guarantee that we can provide in the framework.

I don't think we should try to guarantee bubbling behavior with regards to the rest of the DOM environment since we may choose to put everything at the top level (window/document/body) or in the nearest parent.

For that reason I think that this needs to move into React (even though the implementation is more difficult). :(

[sophiebits]

@sebmarkbage How can we make things like EnterLeaveEventPlugin work with nested roots? The current dispatching strategy traverses the React tree which means that it can't happen properly if the start and end points are in different roots.

[syranide]

@sebmarkbage #2043 is a rather simple solution to making React play nice with other libraries (and other instances of React). It's also imaginable that you could "omit" the listeners for a specific nested root (for performance reasons) if you know it's nested inside a compatible React instance.

Without that React does not play nice with other libraries at all, which is something we currently hype quite a bit (but doesn't quite live up to IMHO).

But to be clear, I will most likely only ever use React for full page apps so I'm not really affected by this one way or the other.

[syranide]

It would be useful to have bubbling of events follow a different order than the DOM. E.g. Layers that are technically part of a component but gets rendered outside of the component. We should have more explicit and defined behavior for how you can nest renderComponent calls that render to different targets.

Do you really need that? I would imagine that layers would simply stop propagation of all events when they're leaving the root of the layer (this would be compatible with HTML). Or what's the goal?

[sophiebits]

We're not going to take this in its current state because the abstraction is a little too leaky, but I've noted on #3210 that we'd like to find a good solution here. Thanks for sending this in!