React: onChange doesn't fire if input re-renders due to a setState() in a non-React capture phase listener

Created on 17 Aug 2018  Â·  10Comments  Â·  Source: facebook/react

Extracting from https://github.com/facebook/react/issues/12643.

This issue has always been in React. I can reproduce it up to React 0.11. However it's probably extremely rare in practice and isn't worth fixing. I'm just filing this for posterity.

Here is a minimal example.

class App extends React.Component {
  state = {value: ''}
  handleChange = (e) => {
    this.setState({
      value: e.target.value
    });
  }
  componentDidMount() {
    document.addEventListener(
      "input",
      () => {
        // COMMENT OUT THIS LINE TO FIX:
        this.setState({});
      },
      true
    );
  }
  render() {
    return (
      <div>
        <input
          value={this.state.value}
          onChange={this.handleChange}
        />
      </div>
    );
  }
}

ReactDOM.render(<App />, document.getElementById("container"));

Typing doesn't work — unless I comment out that setState call in the capture phase listener.

Say the input is empty and we're typing a.

What happens here is that setState({}) in the capture phase non-React listener runs first. When re-rendering due to that first empty setState({}), input props still contain the old value ("") while the DOM node's value is new ("a"). They're not equal, so we'll set the DOM node value to "" (according to the props) and remember "" as the current value.

screen shot 2018-08-17 at 1 08 42 am

Then, ChangeEventPlugin tries to decide whether to emit a change event. It asks the tracker whether the value has changed. The tracker compares the presumably "new" node.value (it's "" — we've just set it earlier!) with the lastValue it has stored (also "" — and also just updated). No changes!

screen shot 2018-08-17 at 1 10 59 am

Our "a" update is lost. We never get the change event, and never actually get a chance to set the correct state.

DOM React Core Team Bug

Most helpful comment

Oh man, I wish I'd stumbled across this thread earlier - was banging my head against this issue for a while and thought I was going insane.

I managed to reproduce my simplified use case here.

I have an application where I've started to add in keyboard shortcuts, hence why I was making use of document.addEventListener as well as other form inputs on the page.

What I was seeing in my app while I was trying to debug this really confused me. When I added debugger statements within the relevant useEffect hook I could see the text appear in the input then disappear within the useEffect hook's cleanup function - all without firing the input's onChange.

While I appreciate that it might not be pragmatic to try and fix what might be considered an "extremely rare" issue; how realistic would it be to add a warning for users about this potential conflict? Maybe even just adding a "N.B." to the documentation?

When I searched for "addEventListener" on the reactjs.org site it took me to Handling Events. Maybe a note here could save people some hassle? Or making a "Gotchas" page for known edge-case issues like this that are known but aren't going to be fixed anytime soon?

In any case, thanks for filing this issue in the first place - it gave me some much needed closure!

All 10 comments

@gaearon I would love to take this one whats the difficulty level?

I’m not sure but since it has always been broken I guess it’s not the easiest one. I’m not even sure it can be solved nicely.

@gaearon Can it be solved by passing the input value at that moment along before running setState?

https://github.com/facebook/react/blob/672e859d311536ddf5f09fb47dcb84007703aa65/packages/react-reconciler/src/ReactFiberClassComponent.js#L174-L190

Maybe, I haven't dived deep into it. Send a PR?

Sorry I can't figure out another way but add another snapshot argument, which is ugly... so let it be

Instead of using this onChange={this.handleChange}, you can use onChange={event => this.handleChange(event.target.value)} />

The issue here is not only because of non-React capture phase listener . I have a separate use case reproducing the same issue, since react makes batch updates to state once per event handler so if multiple events are attached (eg onKeydown and onChange) here onKeydown handler triggers a state update using the useReducer hook which in turn triggers a re-render thus re-setting the updated/changed DOM node value to what is present in the state thus not firing onChange event at all.
These two changes worked for me

  1. replacing the onKeydown with onKeyup thus triggering it after the onChange event
  2. handling the keyDown handler in an async way, refer to this for more details https://reactjs.org/docs/events.html#event-pooling

onKeyDown = {(e) => { e.persist(); setTimeout(() => { this.onKeyDown(e) }, 0); }}

Oh man, I wish I'd stumbled across this thread earlier - was banging my head against this issue for a while and thought I was going insane.

I managed to reproduce my simplified use case here.

I have an application where I've started to add in keyboard shortcuts, hence why I was making use of document.addEventListener as well as other form inputs on the page.

What I was seeing in my app while I was trying to debug this really confused me. When I added debugger statements within the relevant useEffect hook I could see the text appear in the input then disappear within the useEffect hook's cleanup function - all without firing the input's onChange.

While I appreciate that it might not be pragmatic to try and fix what might be considered an "extremely rare" issue; how realistic would it be to add a warning for users about this potential conflict? Maybe even just adding a "N.B." to the documentation?

When I searched for "addEventListener" on the reactjs.org site it took me to Handling Events. Maybe a note here could save people some hassle? Or making a "Gotchas" page for known edge-case issues like this that are known but aren't going to be fixed anytime soon?

In any case, thanks for filing this issue in the first place - it gave me some much needed closure!

I think I have stumbled across the same issue but with a different use case:
https://codesandbox.io/s/onchange-fails-to-fire-ry31c

What's different to the example above:

  • synthetic events used only (non-React listeners are NOT used)
  • combined use of onKeyDown, onChange (with useEffect)

I would have thought this use-case would be more commonly struck? It's actually the second time I've struck it working on the same app. The first was a similar implementation to the above example from @gaearon . Perhaps more common than first thought?

I got this issue on a requirement where the select file dialog should automatically open when the form is opened/rendered/mounted (like an autofocus behaviour). I can't avoid the form re-render due to other code's part. In this case, the select file dialog opens but the input file onChange event doesn't trigger when the user confirms the select the files.

Edit: in case someone is having the same required and is looking for a solution, I did a workaround by creating a second input outside the form.

Was this page helpful?
0 / 5 - 0 ratings