From a930f09dfe6f96347b035e6500b3b13e36e3c7b2 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 7 Dec 2016 18:51:21 -0800 Subject: [PATCH] Give setState callbacks componentWillUpdate semantics This matches the behavior in Fiber. Normally we would change Fiber to match Stack to minimize breaking changes for the initial release. However, in this case it would require too large a compromise to change Fiber to act like Stack. --- scripts/fiber/tests-failing.txt | 3 --- scripts/fiber/tests-passing.txt | 3 +++ ...ReactCompositeComponentNestedState-test.js | 2 +- .../reconciler/ReactCompositeComponent.js | 24 +++++++++++++++++++ .../shared/stack/reconciler/ReactUpdates.js | 15 ------------ 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 8d705c30e8..254abf87b2 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -82,9 +82,6 @@ src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js * should warn about `setState` in getChildContext * should update refs if shouldComponentUpdate gives false -src/renderers/shared/shared/__tests__/ReactCompositeComponentNestedState-test.js -* should provide up to date values for props - src/renderers/shared/shared/__tests__/ReactCompositeComponentState-test.js * should update state when called from child cWRP diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 002f3c042f..e48c182712 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1323,6 +1323,9 @@ src/renderers/shared/shared/__tests__/ReactCompositeComponentDOMMinimalism-test. * should not render extra nodes for non-interpolated text * should not render extra nodes for non-interpolated text +src/renderers/shared/shared/__tests__/ReactCompositeComponentNestedState-test.js +* should provide up to date values for props + src/renderers/shared/shared/__tests__/ReactCompositeComponentState-test.js * should support setting state * should call componentDidUpdate of children first diff --git a/src/renderers/shared/shared/__tests__/ReactCompositeComponentNestedState-test.js b/src/renderers/shared/shared/__tests__/ReactCompositeComponentNestedState-test.js index b44bac3139..1c95f72293 100644 --- a/src/renderers/shared/shared/__tests__/ReactCompositeComponentNestedState-test.js +++ b/src/renderers/shared/shared/__tests__/ReactCompositeComponentNestedState-test.js @@ -111,8 +111,8 @@ describe('ReactCompositeComponentNestedState-state', () => { ['setState-this', 'dark blue', 'blue'], ['setState-args', 'dark blue', 'green'], ['render', 'light green', 'green'], - ['parent-after-setState', 'green'], ['after-setState', 'light green', 'green'], + ['parent-after-setState', 'green'], ]); }); }); diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index 2a15d71383..0064b18d3d 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -780,6 +780,16 @@ var ReactCompositeComponent = { this._context ); } else { + var callbacks = this._pendingCallbacks; + this._pendingCallbacks = null; + if (callbacks) { + for (var j = 0; j < callbacks.length; j++) { + transaction.getReactMountReady().enqueue( + callbacks[j], + this.getPublicInstance() + ); + } + } this._updateBatchNumber = null; } }, @@ -848,6 +858,11 @@ var ReactCompositeComponent = { } } + // If updating happens to enqueue any new updates, we shouldn't execute new + // callbacks until the next render happens, so stash the callbacks first. + var callbacks = this._pendingCallbacks; + this._pendingCallbacks = null; + var nextState = this._processPendingState(nextProps, nextContext); var shouldUpdate = true; @@ -901,6 +916,15 @@ var ReactCompositeComponent = { inst.state = nextState; inst.context = nextContext; } + + if (callbacks) { + for (var j = 0; j < callbacks.length; j++) { + transaction.getReactMountReady().enqueue( + callbacks[j], + this.getPublicInstance() + ); + } + } }, _processPendingState: function(props, context) { diff --git a/src/renderers/shared/stack/reconciler/ReactUpdates.js b/src/renderers/shared/stack/reconciler/ReactUpdates.js index 2e44516430..8971a7c357 100644 --- a/src/renderers/shared/stack/reconciler/ReactUpdates.js +++ b/src/renderers/shared/stack/reconciler/ReactUpdates.js @@ -135,12 +135,6 @@ function runBatchedUpdates(transaction) { // that performUpdateIfNecessary is a noop. var component = dirtyComponents[i]; - // If performUpdateIfNecessary happens to enqueue any new updates, we - // shouldn't execute the callbacks until the next render happens, so - // stash the callbacks first - var callbacks = component._pendingCallbacks; - component._pendingCallbacks = null; - var markerName; if (ReactFeatureFlags.logTopLevelRenders) { var namedComponent = component; @@ -161,15 +155,6 @@ function runBatchedUpdates(transaction) { if (markerName) { console.timeEnd(markerName); } - - if (callbacks) { - for (var j = 0; j < callbacks.length; j++) { - transaction.reconcileTransaction.getReactMountReady().enqueue( - callbacks[j], - component.getPublicInstance() - ); - } - } } }