diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index ff1368fe6c..1f74d76d6f 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1318,6 +1318,7 @@ src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js * should support objects with prototypes as state * should not warn about unmounting during unmounting * should only call componentWillUnmount once +* prepares new child before unmounting old src/renderers/shared/shared/__tests__/ReactCompositeComponentDOMMinimalism-test.js * should not render extra nodes for non-interpolated text @@ -1405,6 +1406,7 @@ src/renderers/shared/shared/__tests__/ReactMultiChild-test.js * should NOT replace children with different owners * should replace children with different keys * should reorder bailed-out children +* prepares new children before unmounting old src/renderers/shared/shared/__tests__/ReactMultiChildReconcile-test.js * should reset internal state if removed then readded in an array diff --git a/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js index 874bdbb296..058323de9b 100644 --- a/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js @@ -1358,4 +1358,45 @@ describe('ReactCompositeComponent', () => { expect(count).toBe(1); }); + it('prepares new child before unmounting old', () => { + var log = []; + + class Spy extends React.Component { + componentWillMount() { + log.push(this.props.name + ' componentWillMount'); + } + render() { + log.push(this.props.name + ' render'); + return
; + } + componentDidMount() { + log.push(this.props.name + ' componentDidMount'); + } + componentWillUnmount() { + log.push(this.props.name + ' componentWillUnmount'); + } + } + + class Wrapper extends React.Component { + render() { + return ; + } + } + + var container = document.createElement('div'); + ReactDOM.render(, container); + ReactDOM.render(, container); + + expect(log).toEqual([ + 'A componentWillMount', + 'A render', + 'A componentDidMount', + + 'B componentWillMount', + 'B render', + 'A componentWillUnmount', + 'B componentDidMount', + ]); + }); + }); diff --git a/src/renderers/shared/shared/__tests__/ReactMultiChild-test.js b/src/renderers/shared/shared/__tests__/ReactMultiChild-test.js index e2d14a7cc3..b1289ebd9b 100644 --- a/src/renderers/shared/shared/__tests__/ReactMultiChild-test.js +++ b/src/renderers/shared/shared/__tests__/ReactMultiChild-test.js @@ -18,11 +18,13 @@ describe('ReactMultiChild', () => { var React; var ReactDOM; + var ReactDOMFeatureFlags; beforeEach(() => { jest.resetModuleRegistry(); React = require('React'); ReactDOM = require('ReactDOM'); + ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); }); describe('reconciliation', () => { @@ -254,4 +256,78 @@ describe('ReactMultiChild', () => { ReactDOM.render(, container); expect(container.textContent).toBe('EHCjpdTUuiybDvhRJwZt'); }); + + it('prepares new children before unmounting old', () => { + var log = []; + + class Spy extends React.Component { + componentWillMount() { + log.push(this.props.name + ' componentWillMount'); + } + render() { + log.push(this.props.name + ' render'); + return
; + } + componentDidMount() { + log.push(this.props.name + ' componentDidMount'); + } + componentWillUnmount() { + log.push(this.props.name + ' componentWillUnmount'); + } + } + + // These are reference-unequal so they will be swapped even if they have + // matching keys + var SpyA = (props) => ; + var SpyB = (props) => ; + + var container = document.createElement('div'); + ReactDOM.render( +
+ + +
, + container + ); + ReactDOM.render( +
+ + +
, + container + ); + + expect(log).toEqual([ + 'oneA componentWillMount', + 'oneA render', + 'twoA componentWillMount', + 'twoA render', + 'oneA componentDidMount', + 'twoA componentDidMount', + + ...( + ReactDOMFeatureFlags.useFiber ? + [ + 'oneB componentWillMount', + 'oneB render', + 'twoB componentWillMount', + 'twoB render', + 'oneA componentWillUnmount', + 'twoA componentWillUnmount', + ] : + [ + 'oneB componentWillMount', + 'oneB render', + 'oneA componentWillUnmount', + 'twoB componentWillMount', + 'twoB render', + 'twoA componentWillUnmount', + ] + ), + + 'oneB componentDidMount', + 'twoB componentDidMount', + ]); + }); + }); diff --git a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js index dc58ff648e..f1ad0189bf 100644 --- a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js +++ b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js @@ -11,10 +11,11 @@ 'use strict'; +var KeyEscapeUtils = require('KeyEscapeUtils'); +var ReactFeatureFlags = require('ReactFeatureFlags'); var ReactReconciler = require('ReactReconciler'); var instantiateReactComponent = require('instantiateReactComponent'); -var KeyEscapeUtils = require('KeyEscapeUtils'); var shouldUpdateReactComponent = require('shouldUpdateReactComponent'); var traverseAllChildren = require('traverseAllChildren'); var warning = require('warning'); @@ -144,7 +145,10 @@ var ReactChildReconciler = { ); nextChildren[name] = prevChild; } else { - if (prevChild) { + if ( + !ReactFeatureFlags.prepareNewChildrenBeforeUnmountInStack && + prevChild + ) { removedNodes[name] = ReactReconciler.getHostNode(prevChild); ReactReconciler.unmountComponent( prevChild, @@ -166,6 +170,17 @@ var ReactChildReconciler = { selfDebugID ); mountImages.push(nextChildMountImage); + if ( + ReactFeatureFlags.prepareNewChildrenBeforeUnmountInStack && + prevChild + ) { + removedNodes[name] = ReactReconciler.getHostNode(prevChild); + ReactReconciler.unmountComponent( + prevChild, + false, /* safely */ + false /* skipLifecycle */ + ); + } } } // Unmount children that are no longer present. diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index 0064b18d3d..a4c3ee9c09 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -15,6 +15,7 @@ var React = require('React'); var ReactComponentEnvironment = require('ReactComponentEnvironment'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactErrorUtils = require('ReactErrorUtils'); +var ReactFeatureFlags = require('ReactFeatureFlags'); var ReactInstanceMap = require('ReactInstanceMap'); var ReactInstrumentation = require('ReactInstrumentation'); var ReactNodeTypes = require('ReactNodeTypes'); @@ -1109,11 +1110,14 @@ var ReactCompositeComponent = { ); } else { var oldHostNode = ReactReconciler.getHostNode(prevComponentInstance); - ReactReconciler.unmountComponent( - prevComponentInstance, - safely, - false /* skipLifecycle */ - ); + + if (!ReactFeatureFlags.prepareNewChildrenBeforeUnmountInStack) { + ReactReconciler.unmountComponent( + prevComponentInstance, + safely, + false /* skipLifecycle */ + ); + } var nodeType = ReactNodeTypes.getType(nextRenderedElement); this._renderedNodeType = nodeType; @@ -1132,6 +1136,14 @@ var ReactCompositeComponent = { debugID ); + if (ReactFeatureFlags.prepareNewChildrenBeforeUnmountInStack) { + ReactReconciler.unmountComponent( + prevComponentInstance, + safely, + false /* skipLifecycle */ + ); + } + if (__DEV__) { if (debugID !== 0) { var childDebugIDs = child._debugID !== 0 ? [child._debugID] : []; diff --git a/src/renderers/shared/utils/ReactFeatureFlags.js b/src/renderers/shared/utils/ReactFeatureFlags.js index 5b3bb1b457..5bd9651aed 100644 --- a/src/renderers/shared/utils/ReactFeatureFlags.js +++ b/src/renderers/shared/utils/ReactFeatureFlags.js @@ -17,6 +17,7 @@ var ReactFeatureFlags = { // render (both initial renders and updates). Useful when looking at prod-mode // timeline profiles in Chrome, for example. logTopLevelRenders: false, + prepareNewChildrenBeforeUnmountInStack: true, }; module.exports = ReactFeatureFlags; diff --git a/src/renderers/testing/__tests__/ReactTestRenderer-test.js b/src/renderers/testing/__tests__/ReactTestRenderer-test.js index 2333095d96..8135d164da 100644 --- a/src/renderers/testing/__tests__/ReactTestRenderer-test.js +++ b/src/renderers/testing/__tests__/ReactTestRenderer-test.js @@ -200,8 +200,8 @@ describe('ReactTestRenderer', () => { expect(log).toEqual([ 'render Foo', 'mount Foo', - 'unmount Foo', 'render Bar', + 'unmount Foo', 'mount Bar', 'unmount Bar', ]);