From 035eb3d4ddb027be226acdb5e72b99f0f1ed89bc Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Thu, 9 Nov 2017 16:41:07 -0200 Subject: [PATCH 01/15] Create test to verify ReactShallowRenderer bug (#11496) --- .../__tests__/ReactShallowRenderer-test.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index b5c093f91e6dc..6d8b9f482e1cd 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -845,6 +845,34 @@ describe('ReactShallowRenderer', () => { expect(result).toEqual(
baz:bar
); }); + it('this.state should be updated on setState callback inside componentWillMount', function() { + let stateSuccessfullyUpdated = false; + + class Component extends React.Component { + constructor(props, context) { + super(props, context); + this.state = { + hasUpdatedState: false, + }; + } + + componentWillMount() { + this.setState( + {hasUpdatedState: true}, + () => (stateSuccessfullyUpdated = this.state.hasUpdatedState), + ); + } + + render() { + return
{this.props.children}
; + } + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + expect(stateSuccessfullyUpdated).toBe(true); + }); + it('throws usefully when rendering badly-typed elements', () => { spyOn(console, 'error'); const shallowRenderer = createRenderer(); From 856cb2ba0d51f21ec345db6693590d1a44218303 Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Thu, 9 Nov 2017 16:44:36 -0200 Subject: [PATCH 02/15] Fix ReactShallowRenderer callback bug on componentWillMount (#11496) --- .../src/ReactShallowRenderer.js | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 1958f43d7bd79..bd2f318d95aed 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -44,7 +44,7 @@ class ReactShallowRenderer { 'ReactShallowRenderer render(): Invalid component element.%s', typeof element === 'function' ? ' Instead of passing a component class, make sure to instantiate ' + - 'it by passing it to React.createElement.' + 'it by passing it to React.createElement.' : '', ); // Show a special message for host elements since it's a common case. @@ -139,6 +139,7 @@ class ReactShallowRenderer { } this._rendered = this._instance.render(); + this._updater._invokeCallback(); // Intentionally do not call componentDidMount() // because DOM refs are not available. } @@ -183,6 +184,7 @@ class ReactShallowRenderer { if (shouldUpdate) { this._rendered = this._instance.render(); + this._updater._invokeCallback(); } // Intentionally do not call componentDidUpdate() // because DOM refs are not available. @@ -192,6 +194,21 @@ class ReactShallowRenderer { class Updater { constructor(renderer) { this._renderer = renderer; + this._callback = null; + this._publicInstance = null; + } + + _updateCallback(callback, publicInstance) { + if (typeof callback === 'function') { + this._callback = callback; + this._publicInstance = publicInstance; + } + } + + _invokeCallback() { + if (typeof this._callback === 'function' && this._publicInstance) { + this._callback.call(this._publicInstance); + } } isMounted(publicInstance) { @@ -199,24 +216,19 @@ class Updater { } enqueueForceUpdate(publicInstance, callback, callerName) { + this._updateCallback(callback, publicInstance); this._renderer._forcedUpdate = true; this._renderer.render(this._renderer._element, this._renderer._context); - - if (typeof callback === 'function') { - callback.call(publicInstance); - } } enqueueReplaceState(publicInstance, completeState, callback, callerName) { + this._updateCallback(callback, publicInstance); this._renderer._newState = completeState; this._renderer.render(this._renderer._element, this._renderer._context); - - if (typeof callback === 'function') { - callback.call(publicInstance); - } } enqueueSetState(publicInstance, partialState, callback, callerName) { + this._updateCallback(callback, publicInstance); const currentState = this._renderer._newState || publicInstance.state; if (typeof partialState === 'function') { @@ -229,10 +241,6 @@ class Updater { }; this._renderer.render(this._renderer._element, this._renderer._context); - - if (typeof callback === 'function') { - callback.call(publicInstance); - } } } From a269747b927f442ac68c78011737aa91b4981c48 Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Thu, 9 Nov 2017 17:45:03 -0200 Subject: [PATCH 03/15] Improve fnction naming and clean up queued callback before call --- .../react-test-renderer/src/ReactShallowRenderer.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index bd2f318d95aed..cfa8cc9a7a1f4 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -198,7 +198,7 @@ class Updater { this._publicInstance = null; } - _updateCallback(callback, publicInstance) { + _enqueueCallback(callback, publicInstance) { if (typeof callback === 'function') { this._callback = callback; this._publicInstance = publicInstance; @@ -207,7 +207,10 @@ class Updater { _invokeCallback() { if (typeof this._callback === 'function' && this._publicInstance) { - this._callback.call(this._publicInstance); + const {_callback, _publicInstance} = this; + this._callback = null; + this._publicInstance = null; + _callback.call(_publicInstance); } } @@ -216,19 +219,19 @@ class Updater { } enqueueForceUpdate(publicInstance, callback, callerName) { - this._updateCallback(callback, publicInstance); + this._enqueueCallback(callback, publicInstance); this._renderer._forcedUpdate = true; this._renderer.render(this._renderer._element, this._renderer._context); } enqueueReplaceState(publicInstance, completeState, callback, callerName) { - this._updateCallback(callback, publicInstance); + this._enqueueCallback(callback, publicInstance); this._renderer._newState = completeState; this._renderer.render(this._renderer._element, this._renderer._context); } enqueueSetState(publicInstance, partialState, callback, callerName) { - this._updateCallback(callback, publicInstance); + this._enqueueCallback(callback, publicInstance); const currentState = this._renderer._newState || publicInstance.state; if (typeof partialState === 'function') { From 11b931d3b5e386e2fe164d8c7b2494081beb661e Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Fri, 10 Nov 2017 10:39:28 -0200 Subject: [PATCH 04/15] Run prettier on ReactShallowRenderer.js --- packages/react-test-renderer/src/ReactShallowRenderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index cfa8cc9a7a1f4..c63e1b0070b42 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -44,7 +44,7 @@ class ReactShallowRenderer { 'ReactShallowRenderer render(): Invalid component element.%s', typeof element === 'function' ? ' Instead of passing a component class, make sure to instantiate ' + - 'it by passing it to React.createElement.' + 'it by passing it to React.createElement.' : '', ); // Show a special message for host elements since it's a common case. From ae88abdac328ede84bf2ec9c8ca0bbf93533a7dd Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Fri, 10 Nov 2017 13:31:51 -0200 Subject: [PATCH 05/15] Consolidate callback call on ReactShallowRenderer.js --- packages/react-test-renderer/src/ReactShallowRenderer.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index c63e1b0070b42..bf7061f888124 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -103,6 +103,7 @@ class ReactShallowRenderer { } this._rendering = false; + this._updater._invokeCallback(); return this.getRenderOutput(); } @@ -139,7 +140,6 @@ class ReactShallowRenderer { } this._rendered = this._instance.render(); - this._updater._invokeCallback(); // Intentionally do not call componentDidMount() // because DOM refs are not available. } @@ -184,7 +184,6 @@ class ReactShallowRenderer { if (shouldUpdate) { this._rendered = this._instance.render(); - this._updater._invokeCallback(); } // Intentionally do not call componentDidUpdate() // because DOM refs are not available. From 87bf19ff067ffa6311245fdc45731d42466ff594 Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Mon, 13 Nov 2017 11:14:36 -0200 Subject: [PATCH 06/15] Ensure callback behavior is similar between ReactDOM and ReactShallowRenderer --- .../react-dom/src/__tests__/ReactDOM-test.js | 41 +++++++++++++++++++ .../__tests__/ReactShallowRenderer-test.js | 32 ++++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 9caa898ff1c9b..62a79ea021d2e 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -338,4 +338,45 @@ describe('ReactDOM', () => { ]; expect(actual).toEqual(expected); }); + + it('should call the setState callback even if shouldComponentUpdate = false', () => { + const mockFn = jest.fn().mockReturnValue(false); + let setState, getState; + + const div = document.createElement('div'); + document.body.appendChild(div); + + class Component extends React.Component { + constructor(props, context) { + super(props, context); + this.state = { + hasUpdatedState: false, + }; + } + + componentWillMount() { + setState = (newState, callback) => this.setState(newState, callback); + getState = () => this.state; + } + + shouldComponentUpdate() { + return mockFn(); + } + + render() { + return
{this.state.hasUpdatedState}
; + } + } + + ReactDOM.render(, div); + + expect(setState).toBeDefined(); + expect(getState).toBeDefined(); + expect(mockFn).not.toBeCalled(); + + setState({hasUpdatedState: true}, () => { + expect(mockFn).toBeCalled(); + expect(getState().hasUpdatedState).toBe(true); + }); + }); }); diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 6d8b9f482e1cd..a7f1850b5d610 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -845,7 +845,7 @@ describe('ReactShallowRenderer', () => { expect(result).toEqual(
baz:bar
); }); - it('this.state should be updated on setState callback inside componentWillMount', function() { + it('this.state should be updated on setState callback inside componentWillMount', () => { let stateSuccessfullyUpdated = false; class Component extends React.Component { @@ -873,6 +873,36 @@ describe('ReactShallowRenderer', () => { expect(stateSuccessfullyUpdated).toBe(true); }); + it('should call the setState callback even if shouldComponentUpdate = false', () => { + const mockFn = jest.fn().mockReturnValue(false); + + class Component extends React.Component { + constructor(props, context) { + super(props, context); + this.state = { + hasUpdatedState: false, + }; + } + + shouldComponentUpdate() { + return mockFn(); + } + + render() { + return
{this.state.hasUpdatedState}
; + } + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + + const mountedInstance = shallowRenderer.getMountedInstance(); + mountedInstance.setState({hasUpdatedState: true}, () => { + expect(mockFn).toBeCalled(); + expect(mountedInstance.state.hasUpdatedState).toBe(true); + }); + }); + it('throws usefully when rendering badly-typed elements', () => { spyOn(console, 'error'); const shallowRenderer = createRenderer(); From 120b3991a05a3db632dc7bcfedbad071735a44cc Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Mon, 13 Nov 2017 13:06:56 -0200 Subject: [PATCH 07/15] Fix Code Review requests (#11507) --- packages/react-dom/src/__tests__/ReactDOM-test.js | 15 ++++++--------- .../src/ReactShallowRenderer.js | 4 ++-- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 62a79ea021d2e..f0c80900427ef 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -341,10 +341,9 @@ describe('ReactDOM', () => { it('should call the setState callback even if shouldComponentUpdate = false', () => { const mockFn = jest.fn().mockReturnValue(false); - let setState, getState; - const div = document.createElement('div'); - document.body.appendChild(div); + + let instance; class Component extends React.Component { constructor(props, context) { @@ -355,8 +354,7 @@ describe('ReactDOM', () => { } componentWillMount() { - setState = (newState, callback) => this.setState(newState, callback); - getState = () => this.state; + instance = this; } shouldComponentUpdate() { @@ -370,13 +368,12 @@ describe('ReactDOM', () => { ReactDOM.render(, div); - expect(setState).toBeDefined(); - expect(getState).toBeDefined(); + expect(instance).toBeDefined(); expect(mockFn).not.toBeCalled(); - setState({hasUpdatedState: true}, () => { + instance.setState({hasUpdatedState: true}, () => { expect(mockFn).toBeCalled(); - expect(getState().hasUpdatedState).toBe(true); + expect(instance.state.hasUpdatedState).toBe(true); }); }); }); diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index bf7061f888124..6eaa3334f0e79 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -103,7 +103,7 @@ class ReactShallowRenderer { } this._rendering = false; - this._updater._invokeCallback(); + this._updater._invokeCallbackIfNecessary(); return this.getRenderOutput(); } @@ -204,7 +204,7 @@ class Updater { } } - _invokeCallback() { + _invokeCallbackIfNecessary() { if (typeof this._callback === 'function' && this._publicInstance) { const {_callback, _publicInstance} = this; this._callback = null; From cfa6552a02cdfcc82e8adc09a7e27a96e27fc145 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 13 Nov 2017 15:26:38 +0000 Subject: [PATCH 08/15] Move test to ReactCompositeComponent --- .../__tests__/ReactCompositeComponent-test.js | 38 +++++++++++++++++++ .../react-dom/src/__tests__/ReactDOM-test.js | 38 ------------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 17b54543fdf02..e9cb5bec76af2 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -1591,6 +1591,44 @@ describe('ReactCompositeComponent', () => { expect(mockArgs.length).toEqual(0); }); + it('should call the setState callback even if shouldComponentUpdate = false', () => { + const mockFn = jest.fn().mockReturnValue(false); + const div = document.createElement('div'); + + let instance; + + class Component extends React.Component { + constructor(props, context) { + super(props, context); + this.state = { + hasUpdatedState: false, + }; + } + + componentWillMount() { + instance = this; + } + + shouldComponentUpdate() { + return mockFn(); + } + + render() { + return
{this.state.hasUpdatedState}
; + } + } + + ReactDOM.render(, div); + + expect(instance).toBeDefined(); + expect(mockFn).not.toBeCalled(); + + instance.setState({hasUpdatedState: true}, () => { + expect(mockFn).toBeCalled(); + expect(instance.state.hasUpdatedState).toBe(true); + }); + }); + it('should return a meaningful warning when constructor is returned', () => { spyOn(console, 'error'); class RenderTextInvalidConstructor extends React.Component { diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index f0c80900427ef..9caa898ff1c9b 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -338,42 +338,4 @@ describe('ReactDOM', () => { ]; expect(actual).toEqual(expected); }); - - it('should call the setState callback even if shouldComponentUpdate = false', () => { - const mockFn = jest.fn().mockReturnValue(false); - const div = document.createElement('div'); - - let instance; - - class Component extends React.Component { - constructor(props, context) { - super(props, context); - this.state = { - hasUpdatedState: false, - }; - } - - componentWillMount() { - instance = this; - } - - shouldComponentUpdate() { - return mockFn(); - } - - render() { - return
{this.state.hasUpdatedState}
; - } - } - - ReactDOM.render(, div); - - expect(instance).toBeDefined(); - expect(mockFn).not.toBeCalled(); - - instance.setState({hasUpdatedState: true}, () => { - expect(mockFn).toBeCalled(); - expect(instance.state.hasUpdatedState).toBe(true); - }); - }); }); From ffa556553578fa1d85d579c6f1e1449c9cf7a2fb Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 13 Nov 2017 15:27:18 +0000 Subject: [PATCH 09/15] Verify the callback gets called --- .../react-dom/src/__tests__/ReactCompositeComponent-test.js | 3 ++- .../src/__tests__/ReactShallowRenderer-test.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index e9cb5bec76af2..6b25e04e3fe62 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -1591,7 +1591,7 @@ describe('ReactCompositeComponent', () => { expect(mockArgs.length).toEqual(0); }); - it('should call the setState callback even if shouldComponentUpdate = false', () => { + it('should call the setState callback even if shouldComponentUpdate = false', done => { const mockFn = jest.fn().mockReturnValue(false); const div = document.createElement('div'); @@ -1626,6 +1626,7 @@ describe('ReactCompositeComponent', () => { instance.setState({hasUpdatedState: true}, () => { expect(mockFn).toBeCalled(); expect(instance.state.hasUpdatedState).toBe(true); + done(); }); }); diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index a7f1850b5d610..a8eb82c2f6c23 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -873,7 +873,7 @@ describe('ReactShallowRenderer', () => { expect(stateSuccessfullyUpdated).toBe(true); }); - it('should call the setState callback even if shouldComponentUpdate = false', () => { + it('should call the setState callback even if shouldComponentUpdate = false', done => { const mockFn = jest.fn().mockReturnValue(false); class Component extends React.Component { @@ -900,6 +900,7 @@ describe('ReactShallowRenderer', () => { mountedInstance.setState({hasUpdatedState: true}, () => { expect(mockFn).toBeCalled(); expect(mountedInstance.state.hasUpdatedState).toBe(true); + done(); }); }); From f242683caf88219dcc8c1595dfd4a8d53e091a0e Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Sun, 19 Nov 2017 18:39:28 -0200 Subject: [PATCH 10/15] Ensure multiple callbacks are correctly handled on ReactShallowRenderer --- .../src/ReactShallowRenderer.js | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 6eaa3334f0e79..d6fe4c01c4b57 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -103,7 +103,7 @@ class ReactShallowRenderer { } this._rendering = false; - this._updater._invokeCallbackIfNecessary(); + this._updater._invokeCallbacks(); return this.getRenderOutput(); } @@ -193,24 +193,23 @@ class ReactShallowRenderer { class Updater { constructor(renderer) { this._renderer = renderer; - this._callback = null; - this._publicInstance = null; + this._callbacks = []; } _enqueueCallback(callback, publicInstance) { - if (typeof callback === 'function') { - this._callback = callback; - this._publicInstance = publicInstance; + if (typeof callback === 'function' && publicInstance) { + this._callbacks.push({ + callback, + publicInstance, + }); } } - _invokeCallbackIfNecessary() { - if (typeof this._callback === 'function' && this._publicInstance) { - const {_callback, _publicInstance} = this; - this._callback = null; - this._publicInstance = null; - _callback.call(_publicInstance); - } + _invokeCallbacks() { + this._callbacks = this._callbacks.filter(({callback, publicInstance}) => { + callback.call(publicInstance); + return false; + }); } isMounted(publicInstance) { From 194ff74577c33ae5ec7ca28f0fdd562442adc546 Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Sun, 19 Nov 2017 19:03:06 -0200 Subject: [PATCH 11/15] Ensure the setState callback is called inside componentWillMount (ReactDOM) --- .../__tests__/ReactCompositeComponent-test.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 6b25e04e3fe62..a20f5ab16d8fe 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -1591,6 +1591,34 @@ describe('ReactCompositeComponent', () => { expect(mockArgs.length).toEqual(0); }); + it('this.state should be updated on setState callback inside componentWillMount', () => { + const div = document.createElement('div'); + let stateSuccessfullyUpdated = false; + + class Component extends React.Component { + constructor(props, context) { + super(props, context); + this.state = { + hasUpdatedState: false, + }; + } + + componentWillMount() { + this.setState( + {hasUpdatedState: true}, + () => (stateSuccessfullyUpdated = this.state.hasUpdatedState), + ); + } + + render() { + return
{this.props.children}
; + } + } + + ReactDOM.render(, div); + expect(stateSuccessfullyUpdated).toBe(true); + }); + it('should call the setState callback even if shouldComponentUpdate = false', done => { const mockFn = jest.fn().mockReturnValue(false); const div = document.createElement('div'); From 2141b8cf8243954be16adcb54c90df256bb7ea2d Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Tue, 21 Nov 2017 22:09:28 -0200 Subject: [PATCH 12/15] Clear ReactShallowRenderer callback queue before actually calling the callbacks --- packages/react-test-renderer/src/ReactShallowRenderer.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index d6fe4c01c4b57..5cf10e7bb935e 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -206,9 +206,11 @@ class Updater { } _invokeCallbacks() { - this._callbacks = this._callbacks.filter(({callback, publicInstance}) => { + const callbacks = this._callbacks; + this._callbacks = []; + + callbacks.forEach(({callback, publicInstance}) => { callback.call(publicInstance); - return false; }); } From b3b078874597d572a53a9c9c87f0dd0b38b8b195 Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Tue, 21 Nov 2017 23:29:30 -0200 Subject: [PATCH 13/15] Add test for multiple callbacks on ReactShallowRenderer --- .../__tests__/ReactShallowRenderer-test.js | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index a8eb82c2f6c23..71b4d9c54a365 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -873,6 +873,38 @@ describe('ReactShallowRenderer', () => { expect(stateSuccessfullyUpdated).toBe(true); }); + it('should handle multiple callbacks', () => { + const mockFn = jest.fn(); + const shallowRenderer = createRenderer(); + + let callbackQueueLength = 0; + + class Component extends React.Component { + constructor(props, context) { + super(props, context); + this.state = { + foo: 'foo', + }; + } + + componentWillMount() { + this.setState({foo: 'bar'}, () => mockFn()); + this.setState({foo: 'foobar'}, () => mockFn()); + + callbackQueueLength = shallowRenderer._updater._callbacks.length; + } + + render() { + return
{this.state.foo}
; + } + } + + shallowRenderer.render(); + + expect(callbackQueueLength).toBe(2); + expect(mockFn.mock.calls.length).toBe(2); + }); + it('should call the setState callback even if shouldComponentUpdate = false', done => { const mockFn = jest.fn().mockReturnValue(false); From 0a586825d02eaa504611a3905360a96a5c0e3ae9 Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Tue, 21 Nov 2017 23:34:41 -0200 Subject: [PATCH 14/15] Ensure the ReactShallowRenderer callback queue is cleared after invoking callbacks --- .../src/__tests__/ReactShallowRenderer-test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 71b4d9c54a365..49533f232a27a 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -903,6 +903,9 @@ describe('ReactShallowRenderer', () => { expect(callbackQueueLength).toBe(2); expect(mockFn.mock.calls.length).toBe(2); + + // Ensure the callback queue is cleared after the callbacks are invoked + expect(shallowRenderer._updater._callbacks.length).toBe(0); }); it('should call the setState callback even if shouldComponentUpdate = false', done => { From ba0e9952e4b9ed828ca584e1f6abbcc8ed700dd0 Mon Sep 17 00:00:00 2001 From: Alexandre Cordeiro Date: Wed, 22 Nov 2017 19:16:28 -0200 Subject: [PATCH 15/15] Remove references to internal fields on ReactShallowRenderer test --- .../src/__tests__/ReactShallowRenderer-test.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 49533f232a27a..0b9ada5e2aadf 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -877,8 +877,6 @@ describe('ReactShallowRenderer', () => { const mockFn = jest.fn(); const shallowRenderer = createRenderer(); - let callbackQueueLength = 0; - class Component extends React.Component { constructor(props, context) { super(props, context); @@ -890,8 +888,6 @@ describe('ReactShallowRenderer', () => { componentWillMount() { this.setState({foo: 'bar'}, () => mockFn()); this.setState({foo: 'foobar'}, () => mockFn()); - - callbackQueueLength = shallowRenderer._updater._callbacks.length; } render() { @@ -901,11 +897,12 @@ describe('ReactShallowRenderer', () => { shallowRenderer.render(); - expect(callbackQueueLength).toBe(2); expect(mockFn.mock.calls.length).toBe(2); // Ensure the callback queue is cleared after the callbacks are invoked - expect(shallowRenderer._updater._callbacks.length).toBe(0); + const mountedInstance = shallowRenderer.getMountedInstance(); + mountedInstance.setState({foo: 'bar'}, () => mockFn()); + expect(mockFn.mock.calls.length).toBe(3); }); it('should call the setState callback even if shouldComponentUpdate = false', done => {