Bad interaction between React "click" event handler and native "click" event handler

[rigdern]

In this repro, I created 2 buttons and only one is rendered at a time. One button's click handler is hooked up to React's synthetic "click" event while the other is hooked up to the native "click" event. Even though only one button is rendered at a time, a single click is triggering both click events.

This bug can be hit by apps that mix React components with non-React controls (e.g. jQuery UI).

Repro Steps

I reproed this in Chrome with React 0.13.2.

1. Go to http://jsfiddle.net/xzucgepn/
2. Click the button labeled "One"

Expected: There's a button rendered labeled "Two"

Actual: The rendered button still has the label "One"

If you open the console, you'll see "goToTwo" and "goToOne" which indicates that the "One" button's click handler ran and then the "Two" button's click handler ran (even though we never saw button "Two"").

[rigdern]

I experimented more and came up with another repro: http://jsfiddle.net/oupyo5ou/1/

In this one, you click on a button with a native "click" handler which causes the button to go away and an input field to be rendered. It results in this exception:

Invariant Violation: ReactMount: Two valid but unequal nodes with the same `data-reactid`: .0

From these 2 repros, it seems something strange is going on when calling setState from within a native "click" handler.

[waldreiter]

It works if you add event.stopPropagation() to the event handler.

[rigdern]

Thanks for the workaround. Is the behavior without the stopPropagation by design? It looks like the bad behavior is being caused by the rerender happening synchronously within the native "click" handler.

[jimfb]

This looks bad.

We don't do a great job interacting with native events. Sebastian wrote a post about this once-upon-a-time, how native and synthetic event systems are, by and large, not interoperable. Even so, I can't see a reason we should fatal like this.

@syranide @spicyj Do you understand what's going wrong here? What a fix might look like?

[sophiebits]

This happens when we handle a click event from a node that's been removed in between when the event is triggered and when React receives it.

@sebmarkbage How should we deal with the case that events fire on detached elements? Should we ignore events for nodes that aren't in the document? I don't think the bug is easily fixable otherwise because we don't have the old event handler after the reconcile that removes that node.

[rigdern]

Here are some additional details about the impact of the bug. It sounds like this bug is likely to trigger when you use a non-React component (e.g. jQuery UI) in a React single page application and the non-React component triggers a navigation within the app.

[sebmarkbage]

@rigdern Thanks for an excellent bug report and good repro case!

@spicyj I think this happens because we use the "root id" to determine which element was clicked. In the repro case there is no "key" as part of the root ID for two reasons:

1. It is not a child of a container (multichild) so it doesn't get a key added to the root ID.
2. It doesn't have a unique key and doesn't need one because it the button is of a different type.

If I make sure that a key is used as part of the root ID the problem is solved:

http://jsfiddle.net/xzucgepn/1/

So the problem in the repro isn't that we're firing on a removed node, it is that we're firing the event on the new node. This would be solved if we used a unique ID per instance to identify event handlers instead of a generate root that is non-unique. Which I think we wanted to do anyway.

Is there another repro that shows a different issue?