I have the following component:
class MyInput extends React.Component {
state = {
value: '',
};
render() {
return (
<input
onChange={(e) => {
this.setState({value: e.target.value});
this.props.onChange(e);
}}
type="text"
/>
);
}
}
My test is as follows:
it('calls onChange before the internal state updates', () => {
const wrapper = mount(
<MyInput
onChange={(e) => {
// This assertion passes as expected.
expect(e.target.value).to.equal('a');
// This assertion fails with a value of 'a', but should pass.
expect(wrapper.instance().state.value).to.equal('');
}}
/>
);
wrapper.find('input').prop('onChange')({target: {value: 'a'}});
});
Enzyme seems to process setState synchronously, so the internal state of the component updates before onChange is called. If I move onChange to be before setState, then state.value is ''.
In this case, setState is asynchronous, so this.props.onChange should be called before the internal state is updated whether the onChange call is before or after the setState call. When I test the behavior in a browser, that's what happens:
<MyInput
onChange={(e) => {
// The following values will not be the same.
console.log(e.target.value);
console.log(this._input.state.value);
}}
ref={(el) => this._input = el}
/>
Webpack/Karma/Mocha/Chai/Enzyme
| library | version
| ---------------- | -------
| Enzyme | 3.3.0
| React | 16.3.1
setState may be asynchronous.
https://reactjs.org/docs/state-and-lifecycle.html#state-updates-may-be-asynchronous
I would say in this case enzyme works as expected.
In this case setState is asynchronous (as demonstrated in the real-world example), but Enzyme treats it as strictly synchronous (as demonstrated by moving the position of the onChange call). That seems wrong.
@ericgio you should move the this.props.onChange call into a setState callback if you want to be 100% sure that it's dispatched after the state has updated.
onChange={e => {
this.setState({ value: e.target.value }, () => {
this.props.onChange(this.state.value);
}
React doesn't make any explicit guarantees about when setState may or may not be asynchronous, so it's important not to rely on it one way or the other.
It might still make sense to make Enzyme consistent here, but ultimately your code shouldn't depend on this behavior.
If React 16.3 changed this to always be async, then that's a change we should make, but I agree with https://github.com/airbnb/enzyme/issues/1608#issuecomment-379896758
you should move the this.props.onChange call into a setState callback if you want to be 100% sure that it's dispatched after the state has updated
@aweary: yup, that's exactly what I'm doing :) The reason I'm raising the issue is that I want to write the test first, but can't get it to fail as expected.
@ljharb setState can still be synchronous in some cases. For example, if you call setState from some async callback outside React's lifecycle method, like setTimeout or an event listener setup manually (example). I'm not sure if this will remain synchronous though, could just be an implementation detail.
I think async-by-default is probably the best strategy.
@aweary from other perspective setState is usually synchronous (at least in 16.2) when used in usual react lifecycle (from componentWillReceiveProps). Since it is may be asynchronous in react docs I would say synchronous implementation is better.
Why does Enzyme make decisions about how setState is processed in the first place?
(I don't have a deep understanding about how either Enzyme or the React test environment is implemented, so sorry if that's a naive question)
How do you expect it to be implemented?
In React docs it is said that state may be updated async, which to my mind means that usually is it being updated syncronously. Enzyme does it in this usual and expected way.
Should it be like Math.random() > 0.5 ? doSync() : doAsync()? 馃槂
@atsikov: You make it sound as though async updates to state are somehow uncommon, which is not true at all. The case in my original post (updating state from an event handler) is a very common use case. Also, your assertion that setState is synchronous in componentWillReceiveProps is wrong, since state is updated later in the lifecycle:
class MyComponent extends React.Component {
state = {
foo: '',
};
componentWillReceiveProps(nextProps) {
console.log(this.state.foo); // ''
this.setState({foo: 'bar'});
console.log(this.state.foo); // ''
}
componentDidUpdate(prevProps, prevState) {
console.log(prevState.foo); // ''
console.log(this.state.foo); // 'bar'
}
...
}
How do you expect it to be implemented?
I would expect Enzyme not to have an opinion about it and simply let React do its thing.
@ericgio enzyme has to have an opinion about it, in order to be able to provide the hooks that it does, and provide adapter interfaces to multiple react versions, or to alternative frameworks.
@aweary what do you recommend be done here?
Maybe there should be some methods that simulates "business" of internal dispatcher. Today I found a bug caused by usage this.state instead of prevState from callback, it is extremely hard to reproduce it in unit test because of synchronous execution of setState.
Most helpful comment
@ericgio you should move the
this.props.onChangecall into asetStatecallback if you want to be 100% sure that it's dispatched after the state has updated.React doesn't make any explicit guarantees about when
setStatemay or may not be asynchronous, so it's important not to rely on it one way or the other.It might still make sense to make Enzyme consistent here, but ultimately your code shouldn't depend on this behavior.