Bug fix - SetState callback called before component state is updated in ReactShallowRenderer (#11507)
* Create test to verify ReactShallowRenderer bug (#11496) * Fix ReactShallowRenderer callback bug on componentWillMount (#11496) * Improve fnction naming and clean up queued callback before call * Run prettier on ReactShallowRenderer.js * Consolidate callback call on ReactShallowRenderer.js * Ensure callback behavior is similar between ReactDOM and ReactShallowRenderer * Fix Code Review requests (#11507) * Move test to ReactCompositeComponent * Verify the callback gets called * Ensure multiple callbacks are correctly handled on ReactShallowRenderer * Ensure the setState callback is called inside componentWillMount (ReactDOM) * Clear ReactShallowRenderer callback queue before actually calling the callbacks * Add test for multiple callbacks on ReactShallowRenderer * Ensure the ReactShallowRenderer callback queue is cleared after invoking callbacks * Remove references to internal fields on ReactShallowRenderer test
Alex Cordeiro committed
Nov 22, 2017 at 20:15 UTC
f114bad09f7a3ddc11265741cb84250fd08d5890
3 files changed
+181
-12
packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
+67
@@ -1634,6 +1634,73 @@ describe('ReactCompositeComponent', () => {
1634
expect(mockArgs.length).toEqual(0);
1635
});
1636
1637
+ it('this.state should be updated on setState callback inside componentWillMount', () => {
1638
+ const div = document.createElement('div');
1639
+ let stateSuccessfullyUpdated = false;
1640
+
1641
+ class Component extends React.Component {
1642
+ constructor(props, context) {
1643
+ super(props, context);
1644
+ this.state = {
1645
+ hasUpdatedState: false,
1646
+ };
1647
+ }
1648
+
1649
+ componentWillMount() {
1650
+ this.setState(
1651
+ {hasUpdatedState: true},
1652
+ () => (stateSuccessfullyUpdated = this.state.hasUpdatedState),
1653
+ );
1654
+ }
1655
+
1656
+ render() {
1657
+ return <div>{this.props.children}</div>;
1658
+ }
1659
+ }
1660
+
1661
+ ReactDOM.render(<Component />, div);
1662
+ expect(stateSuccessfullyUpdated).toBe(true);
1663
+ });
1664
+
1665
+ it('should call the setState callback even if shouldComponentUpdate = false', done => {
1666
+ const mockFn = jest.fn().mockReturnValue(false);
1667
+ const div = document.createElement('div');
1668
+
1669
+ let instance;
1670
+
1671
+ class Component extends React.Component {
1672
+ constructor(props, context) {
1673
+ super(props, context);
1674
+ this.state = {
1675
+ hasUpdatedState: false,
1676
+ };
1677
+ }
1678
+
1679
+ componentWillMount() {
1680
+ instance = this;
1681
+ }
1682
+
1683
+ shouldComponentUpdate() {
1684
+ return mockFn();
1685
+ }
1686
+
1687
+ render() {
1688
+ return <div>{this.state.hasUpdatedState}</div>;
1689
+ }
1690
+ }
1691
+
1692
+ ReactDOM.render(<Component />, div);
1693
+
1694
+ expect(instance).toBeDefined();
1695
+ expect(mockFn).not.toBeCalled();
1696
+
1697
+ instance.setState({hasUpdatedState: true}, () => {
1698
+ expect(mockFn).toBeCalled();
1699
+ expect(instance.state.hasUpdatedState).toBe(true);
1700
+ done();
1701
+ });
1702
+ });
1703
+
1704
it('should return a meaningful warning when constructor is returned', () => {
1705
spyOnDev(console, 'error');
1706
class RenderTextInvalidConstructor extends React.Component {
packages/react-test-renderer/src/ReactShallowRenderer.js
+23
-12
@@ -103,6 +103,7 @@ class ReactShallowRenderer {
103
}
104
105
this._rendering = false;
106
+ this._updater._invokeCallbacks();
107
108
return this.getRenderOutput();
109
}
@@ -192,6 +193,25 @@ class ReactShallowRenderer {
193
class Updater {
194
constructor(renderer) {
195
this._renderer = renderer;
196
+ this._callbacks = [];
197
+ }
198
+
199
+ _enqueueCallback(callback, publicInstance) {
200
+ if (typeof callback === 'function' && publicInstance) {
201
+ this._callbacks.push({
202
+ callback,
203
+ publicInstance,
204
+ });
205
+ }
206
+ }
207
+
208
+ _invokeCallbacks() {
209
+ const callbacks = this._callbacks;
210
+ this._callbacks = [];
211
+
212
+ callbacks.forEach(({callback, publicInstance}) => {
213
+ callback.call(publicInstance);
214
+ });
215
}
216
217
isMounted(publicInstance) {
@@ -199,24 +219,19 @@ class Updater {
219
}
220
221
enqueueForceUpdate(publicInstance, callback, callerName) {
222
+ this._enqueueCallback(callback, publicInstance);
223
this._renderer._forcedUpdate = true;
224
this._renderer.render(this._renderer._element, this._renderer._context);
204
-
205
- if (typeof callback === 'function') {
206
- callback.call(publicInstance);
207
- }
225
}
226
227
enqueueReplaceState(publicInstance, completeState, callback, callerName) {
228
+ this._enqueueCallback(callback, publicInstance);
229
this._renderer._newState = completeState;
230
this._renderer.render(this._renderer._element, this._renderer._context);
213
-
214
- if (typeof callback === 'function') {
215
- callback.call(publicInstance);
216
- }
231
}
232
233
enqueueSetState(publicInstance, partialState, callback, callerName) {
234
+ this._enqueueCallback(callback, publicInstance);
235
const currentState = this._renderer._newState || publicInstance.state;
236
237
if (typeof partialState === 'function') {
@@ -229,10 +244,6 @@ class Updater {
244
};
245
246
this._renderer.render(this._renderer._element, this._renderer._context);
232
-
233
- if (typeof callback === 'function') {
234
- callback.call(publicInstance);
235
- }
247
}
248
}
249
packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js
+91
@@ -849,6 +849,97 @@ describe('ReactShallowRenderer', () => {
849
expect(result).toEqual(<div>baz:bar</div>);
850
});
851
852
+ it('this.state should be updated on setState callback inside componentWillMount', () => {
853
+ let stateSuccessfullyUpdated = false;
854
+
855
+ class Component extends React.Component {
856
+ constructor(props, context) {
857
+ super(props, context);
858
+ this.state = {
859
+ hasUpdatedState: false,
860
+ };
861
+ }
862
+
863
+ componentWillMount() {
864
+ this.setState(
865
+ {hasUpdatedState: true},
866
+ () => (stateSuccessfullyUpdated = this.state.hasUpdatedState),
867
+ );
868
+ }
869
+
870
+ render() {
871
+ return <div>{this.props.children}</div>;
872
+ }
873
+ }
874
+
875
+ const shallowRenderer = createRenderer();
876
+ shallowRenderer.render(<Component />);
877
+ expect(stateSuccessfullyUpdated).toBe(true);
878
+ });
879
+
880
+ it('should handle multiple callbacks', () => {
881
+ const mockFn = jest.fn();
882
+ const shallowRenderer = createRenderer();
883
+
884
+ class Component extends React.Component {
885
+ constructor(props, context) {
886
+ super(props, context);
887
+ this.state = {
888
+ foo: 'foo',
889
+ };
890
+ }
891
+
892
+ componentWillMount() {
893
+ this.setState({foo: 'bar'}, () => mockFn());
894
+ this.setState({foo: 'foobar'}, () => mockFn());
895
+ }
896
+
897
+ render() {
898
+ return <div>{this.state.foo}</div>;
899
+ }
900
+ }
901
+
902
+ shallowRenderer.render(<Component />);
903
+
904
+ expect(mockFn.mock.calls.length).toBe(2);
905
+
906
+ // Ensure the callback queue is cleared after the callbacks are invoked
907
+ const mountedInstance = shallowRenderer.getMountedInstance();
908
+ mountedInstance.setState({foo: 'bar'}, () => mockFn());
909
+ expect(mockFn.mock.calls.length).toBe(3);
910
+ });
911
+
912
+ it('should call the setState callback even if shouldComponentUpdate = false', done => {
913
+ const mockFn = jest.fn().mockReturnValue(false);
914
+
915
+ class Component extends React.Component {
916
+ constructor(props, context) {
917
+ super(props, context);
918
+ this.state = {
919
+ hasUpdatedState: false,
920
+ };
921
+ }
922
+
923
+ shouldComponentUpdate() {
924
+ return mockFn();
925
+ }
926
+
927
+ render() {
928
+ return <div>{this.state.hasUpdatedState}</div>;
929
+ }
930
+ }
931
+
932
+ const shallowRenderer = createRenderer();
933
+ shallowRenderer.render(<Component />);
934
+
935
+ const mountedInstance = shallowRenderer.getMountedInstance();
936
+ mountedInstance.setState({hasUpdatedState: true}, () => {
937
+ expect(mockFn).toBeCalled();
938
+ expect(mountedInstance.state.hasUpdatedState).toBe(true);
939
+ done();
940
+ });
941
+ });
942
+
943
it('throws usefully when rendering badly-typed elements', () => {
944
spyOnDev(console, 'error');
945
const shallowRenderer = createRenderer();