Click handler can get called twice in "non-prod" mode.

[pllee]

I think this is a bug that only happens in "non-prod" mode.

I noticed this using the react-dropzone library but I think I found a simple use case to reproduce the problem. If you check out the fiddle you can see that click handler is called twice even though the element is only clicked once. Here is an example showing it "working and not working"

I believe it has something to do with the outer div's click listener and this chunk of code:

if (true) {
  /**
   * To help development we can get better devtools integration by simulating a
   * real browser event.
   */
  if (typeof window !== 'undefined' && typeof window.dispatchEvent === 'function' && typeof document !== 'undefined' && typeof document.createEvent === 'function') {
    var fakeNode = document.createElement('react');
    ReactErrorUtils.invokeGuardedCallback = function (name, func, a) {
      var boundFunc = func.bind(null, a);
      var evtType = 'react-' + name;
      fakeNode.addEventListener(evtType, boundFunc, false);
      var evt = document.createEvent('Event');
      // $FlowFixMe https://github.com/facebook/flow/issues/2336
      evt.initEvent(evtType, false, false);
      fakeNode.dispatchEvent(evt);
      fakeNode.removeEventListener(evtType, boundFunc, false);
    };
  }
}

I tested this on Chrome/Mac I have also seen this behavior on React 14.8.

[edvinerikson]

changing let clickCount = ++this.state.clickCount; to let clickCount = this.state.clickCount + 1 stops the counter incrementing by two each time.
also I think it might be event bubbling that is happening since you trigger a click in a child which should bubble up.

[Andarist]

Im not really aware of React internals, but from my short debug it seems that the culprit is this line - this.refs.fileInput.open(); in your button's onClick handler.

Its synchronously calling this.refs.inputEl.click();, which in effect triggers top level dispatch of the click event. Your button's onClick handler at this moment is still not finished so probably in some internal queue and when this programatically induced click is being handled it calls both onClick handlers - button's and div's.

If the issue should be resolved, I would call a dibs on it. With some guidance about internals from somebody way more experience in react than I am should be quite easily fixable (I think ;) )

[pllee]

@edvinerikson changing click count does fix that but I just added that to make the issue of it being called twice more obvious. If you look at the logs the button click is called twice.

Yeah it looks like an issue with the event bubbling you can see it working in my "working and not working" example by not invoking the click.

Also to be clear this does not show up in a minified "prod" version of React and I would argue that "dev mode" and "prod" mode should have the same functional behavior.

[aweary]

I need to look at the example closer to say what's happening, but a quick note: ++this.state. clickCount will mutate this.state which isn't likely what you want.

I believe it has something to do with the outer div's click listener and this chunk of code:

This code is used for debugging purposes and isn't likely related. We're not actually dispatching a click event here, we're dispatching a custom event which in turn is how your event handler (and other code) is executed.

[pllee]

@aweary Just an fyi if I comment out the code that I mentioned I don't see the issue in my app which uses 14.8.

[Andarist]

You can see here whats the stack trace for the second dispatchEvent which causes second onClick run.

Also I've verified that in this second run of dispatchEvent caused by programatically clicking on input (this.refs.inputEl.click();) within button's onClick handler when reaching those lines (which are run only in development mode) fakeNode.dispatchEvent(evt); causes both handlers to run.

Sidenote - fakeNode.dispatchEvent(evt); is full native, where evt.type === 'react-click'

Also when changing those lines to:

  open() {
    this.refs.inputEl.click();
    this.refs.inputEl.click();
    this.refs.inputEl.click();
  },

the outcome is

button click 1481645754505
button click 1481645754509
div click 1481645754509
button click 1481645754510
div click 1481645754510
button click 1481645754511
div click 1481645754511

Which proves that each this.refs.inputEl.click(); is causing a pair of button click + div click. And as I said its caused by this synchrosity

      fakeNode.addEventListener(evtType, boundFunc, false); // #1
      var evt = document.createEvent('Event'); 
      // $FlowFixMe https://github.com/facebook/flow/issues/2336
      evt.initEvent(evtType, false, false);
      fakeNode.dispatchEvent(evt); // #2
      fakeNode.removeEventListener(evtType, boundFunc, false); // #3

1. setup event listener for react-click for button
2. dispatch event which synchronously causing new click to happen so meeting 1. again to handle div's click, so effectively 2 listeners for react-click are set up in this moment (1st wasnt released yet) so both listeners are fired up
3. finally removeEventListener for div's onClick and later higher in the call stack button's onClick

Question is - how to properly set up those guarded callbacks in development mode so the code benefit from it but additionally do not fall into this synchronous bug.

[aweary]

For reference, this does not occur in the React 16 alpha: https://jsfiddle.net/6L9hgf2j/

[Andarist]

@aweary I think this issue can be closed then, right?

[Andarist]

For reference - while implementation has changed the simpler fix was introduced in #10296