Describe the bug
Calling setProps on a shallow wrapper results in componentDidUpdate being called twice (instead of once).
To Reproduce
Steps to reproduce the behavior:
Run the test:
import React from 'react';
export default class Dummy extends React.Component {
componentWillReceiveProps(nextProps) {
this.setState({someState: nextProps.myProp});
}
componentDidUpdate() {
this.foo();
}
foo() {}
render() {
return (
<div>
<div>{`myProp: ${this.props.myProp}`}</div>
</div>
);
}
}
import React from 'react';
import {shallow} from 'enzyme';
import Dummy from 'dummy.js';
describe('<Dummy/>', () => {
it('works', () => {
const props = {};
const wrapper = shallow(<Dummy {...props} />);
wrapper.instance().foo = jest.fn();
wrapper.setProps({myProp: 'Prop Value'});
expect(wrapper.instance().foo).toHaveBeenCalledTimes(1);
});
});
Expected behavior
componentDidUpdate should only be called once.
Desktop (please complete the following information):
Smartphone (please complete the following information):
n/a
(wrapper.instance().foo = jest.fn(); isn't doing what you think it is; you need to spy on Dummy.prototype.foo before the initial render)
It seems like the issue is that it's calling cWRP, queueing up a setState, rerendering with the props only, and calling cDU, and then rerendering with the new state, and then calling cDU again.
I presume that React itself is batching up the updates so that this is working as expected - I don't believe we have proper batching support for React 16 just yet.
(cc @koba04)
What happens when you use mount?
@ljharb
Yeah, React doesn't support batched updates on ShallowRendering from v16.
https://github.com/facebook/react/issues/10355
So currently, it's a no-op.
If we support batching updates on shallow to fix this, we have to implement ourselves.
Although that's a lot more work, I think it's probably worth it so that things are consistent between React and enzyme.
@ljharb thanks, mount works. I've edited the title and body to explicitly mention shallow.
I'll go ahead and look into this issue if a fix hasn't already been applied.
@ljharb Added the above test case, but in terms of the fix, has there been much discussion on adding support for batched updates with the ReactSixteenThreeAdapter?
I've been looking at @koba04's implementation of unstable_batchedUpdates
https://github.com/koba04/react/commit/ca78c92635525ad2dfd8012e7e5527a5876ce130,
but would you have any suggestions for moving forward?
@peanutenthusiast not much, but I'd love to add it (ideally separate from bug fixes)
Was out of country for a month or so, but I've come up with a fix for the above test case -- @ljharb are there any other test cases that should be covered? I can send a PR tonight.
Not that i know of; PR away!
Awesome, thanks everyone!
Most helpful comment
@ljharb thanks,
mountworks. I've edited the title and body to explicitly mentionshallow.