From 09b993e582fd26a77f68ea19f74817464aa51aeb Mon Sep 17 00:00:00 2001 From: Brandon Dail Date: Wed, 24 Oct 2018 18:24:46 -0700 Subject: [PATCH 01/14] Resolve defaultProps for Lazy components --- .../src/ReactFiberBeginWork.js | 5 +- .../src/__tests__/ReactLazy-test.internal.js | 104 ++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 67808381bf06a..f4876f56745bb 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -771,7 +771,10 @@ function mountLazyComponent( workInProgress.type = Component; const resolvedTag = (workInProgress.tag = resolveLazyComponentTag(Component)); startWorkTimer(workInProgress); - const resolvedProps = resolveDefaultProps(Component, props); + const resolvedProps = (workInProgress.pendingProps = resolveDefaultProps( + Component, + props, + )); let child; switch (resolvedTag) { case FunctionComponent: { diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 22155831fbef8..dbf169290975b 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -282,6 +282,110 @@ describe('ReactLazy', () => { expect(root).toMatchRenderedOutput('SiblingB'); }); + it('sets defaultProps for all lifecycles', async () => { + const log = []; + class C extends React.Component { + static defaultProps = {text: 'A'}; + // Added to allow gDSFP + state = {}; + + static getDerivedStateFromProps(props) { + ReactTestRenderer.unstable_yield( + `getDerivedStateFromProps: ${props.text}`, + ); + return null; + } + + constructor(props) { + super(props); + ReactTestRenderer.unstable_yield(`constructor: ${this.props.text}`); + } + + componentDidMount() { + ReactTestRenderer.unstable_yield( + `componentDidMount: ${this.props.text}`, + ); + } + + componentDidUpdate() { + ReactTestRenderer.unstable_yield( + `componentDidUpdate: ${this.props.text}`, + ); + } + + componentWillUnmount() { + ReactTestRenderer.unstable_yield( + `componentWillUnmount: ${this.props.text}`, + ); + } + + shouldComponentUpdate(nextProps) { + ReactTestRenderer.unstable_yield( + `shouldComponentUpdate (this.props): ${this.props.text}`, + ); + ReactTestRenderer.unstable_yield( + `shouldComponentUpdate (nextProps): ${nextProps.text}`, + ); + return true; + } + + getSnapshotBeforeUpdate(prevProps) { + console.log(this.props); + ReactTestRenderer.unstable_yield( + `getSnapshotBeforeUpdate (this.props): ${this.props.text}`, + ); + ReactTestRenderer.unstable_yield( + `getSnapshotBeforeUpdate (prevProps): ${prevProps.text}`, + ); + return null; + } + + render() { + return ; + } + } + + const LazyClass = lazy(() => fakeImport(C)); + + const root = ReactTestRenderer.create( + }> + + , + { + unstable_isConcurrent: true, + }, + ); + + expect(root).toFlushAndYield(['Loading...']); + expect(root).toMatchRenderedOutput(null); + + await Promise.resolve(); + + expect(root).toFlushAndYield([ + 'constructor: A', + 'getDerivedStateFromProps: A', + 'A1', // render + 'componentDidMount: A', + ]); + + root.update( + }> + + , + ); + + expect(root).toFlushAndYield([ + 'getDerivedStateFromProps: A', + 'shouldComponentUpdate (this.props): A', + 'shouldComponentUpdate (nextProps): A', + 'A2', // render + 'getSnapshotBeforeUpdate (this.props): undefined', + 'getSnapshotBeforeUpdate (prevProps): A', + 'componentDidUpdate: A', + ]); + expect(root).toMatchRenderedOutput('A2'); + }); + it('includes lazy-loaded component in warning stack', async () => { const LazyFoo = lazy(() => { ReactTestRenderer.unstable_yield('Started loading'); From baacebafe79c24aea777ec44c9ff317c3e5e4c7e Mon Sep 17 00:00:00 2001 From: Brandon Dail Date: Wed, 24 Oct 2018 18:27:20 -0700 Subject: [PATCH 02/14] Make test fail again --- .../react-reconciler/src/__tests__/ReactLazy-test.internal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index dbf169290975b..88d7251016bd4 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -379,7 +379,7 @@ describe('ReactLazy', () => { 'shouldComponentUpdate (this.props): A', 'shouldComponentUpdate (nextProps): A', 'A2', // render - 'getSnapshotBeforeUpdate (this.props): undefined', + 'getSnapshotBeforeUpdate (this.props): A', 'getSnapshotBeforeUpdate (prevProps): A', 'componentDidUpdate: A', ]); From 34b3cb9ca5ae801dccb6a7042bc8cac4150719e1 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 5 Nov 2018 18:45:07 +0000 Subject: [PATCH 03/14] Undo the partial fix --- packages/react-reconciler/src/ReactFiberBeginWork.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index f4876f56745bb..67808381bf06a 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -771,10 +771,7 @@ function mountLazyComponent( workInProgress.type = Component; const resolvedTag = (workInProgress.tag = resolveLazyComponentTag(Component)); startWorkTimer(workInProgress); - const resolvedProps = (workInProgress.pendingProps = resolveDefaultProps( - Component, - props, - )); + const resolvedProps = resolveDefaultProps(Component, props); let child; switch (resolvedTag) { case FunctionComponent: { From 94d1d17909df7d198f5567bd5a6234eb5691ee62 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 5 Nov 2018 18:49:45 +0000 Subject: [PATCH 04/14] Make test output more compact --- .../src/__tests__/ReactLazy-test.internal.js | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 88d7251016bd4..ef831cf6ec87b 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -282,11 +282,10 @@ describe('ReactLazy', () => { expect(root).toMatchRenderedOutput('SiblingB'); }); - it('sets defaultProps for all lifecycles', async () => { + it('sets defaultProps for modern lifecycles', async () => { const log = []; class C extends React.Component { static defaultProps = {text: 'A'}; - // Added to allow gDSFP state = {}; static getDerivedStateFromProps(props) { @@ -307,9 +306,9 @@ describe('ReactLazy', () => { ); } - componentDidUpdate() { + componentDidUpdate(prevProps) { ReactTestRenderer.unstable_yield( - `componentDidUpdate: ${this.props.text}`, + `componentDidUpdate: ${prevProps.text} -> ${this.props.text}`, ); } @@ -321,10 +320,7 @@ describe('ReactLazy', () => { shouldComponentUpdate(nextProps) { ReactTestRenderer.unstable_yield( - `shouldComponentUpdate (this.props): ${this.props.text}`, - ); - ReactTestRenderer.unstable_yield( - `shouldComponentUpdate (nextProps): ${nextProps.text}`, + `shouldComponentUpdate: ${this.props.text} -> ${nextProps.text}`, ); return true; } @@ -332,10 +328,7 @@ describe('ReactLazy', () => { getSnapshotBeforeUpdate(prevProps) { console.log(this.props); ReactTestRenderer.unstable_yield( - `getSnapshotBeforeUpdate (this.props): ${this.props.text}`, - ); - ReactTestRenderer.unstable_yield( - `getSnapshotBeforeUpdate (prevProps): ${prevProps.text}`, + `getSnapshotBeforeUpdate: ${prevProps.text} -> ${this.props.text}`, ); return null; } @@ -364,7 +357,7 @@ describe('ReactLazy', () => { expect(root).toFlushAndYield([ 'constructor: A', 'getDerivedStateFromProps: A', - 'A1', // render + 'A1', 'componentDidMount: A', ]); @@ -376,12 +369,10 @@ describe('ReactLazy', () => { expect(root).toFlushAndYield([ 'getDerivedStateFromProps: A', - 'shouldComponentUpdate (this.props): A', - 'shouldComponentUpdate (nextProps): A', - 'A2', // render - 'getSnapshotBeforeUpdate (this.props): A', - 'getSnapshotBeforeUpdate (prevProps): A', - 'componentDidUpdate: A', + 'shouldComponentUpdate: A -> A', + 'A2', + 'getSnapshotBeforeUpdate: A -> A', + 'componentDidUpdate: A -> A', ]); expect(root).toMatchRenderedOutput('A2'); }); From 89ec002673e541c0c8fae781a2075096eae2c4f2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 5 Nov 2018 19:01:44 +0000 Subject: [PATCH 05/14] Add a separate failing test for sync mode --- .../src/__tests__/ReactLazy-test.internal.js | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index ef831cf6ec87b..d503cbefaeac4 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -377,6 +377,68 @@ describe('ReactLazy', () => { expect(root).toMatchRenderedOutput('A2'); }); + it('sets defaultProps for legacy lifecycles', async () => { + const log = []; + class C extends React.Component { + static defaultProps = {text: 'A'}; + state = {}; + + UNSAFE_componentWillMount() { + ReactTestRenderer.unstable_yield( + `UNSAFE_componentWillMount: ${this.props.text}`, + ); + } + + UNSAFE_componentWillUpdate(nextProps) { + ReactTestRenderer.unstable_yield( + `UNSAFE_componentWillUpdate: ${this.props.text} -> ${nextProps.text}`, + ); + } + + UNSAFE_componentWillReceiveProps(nextProps) { + ReactTestRenderer.unstable_yield( + `UNSAFE_componentWillReceiveProps: ${this.props.text} -> ${ + nextProps.text + }`, + ); + } + + render() { + return ; + } + } + + const LazyClass = lazy(() => fakeImport(C)); + + const root = ReactTestRenderer.create( + }> + + , + ); + + expect(ReactTestRenderer).toHaveYielded(['Loading...']); + expect(root).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('Loading...'); + + await Promise.resolve(); + + root.update( + }> + + , + ); + + expect(ReactTestRenderer).toHaveYielded([ + 'UNSAFE_componentWillMount: A', + 'A1', + 'UNSAFE_componentWillReceiveProps: A -> A', + 'UNSAFE_componentWillUpdate: A -> A', + 'A2', + ]); + expect(root).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('A2'); + }); + it('includes lazy-loaded component in warning stack', async () => { const LazyFoo = lazy(() => { ReactTestRenderer.unstable_yield('Started loading'); From c0b2b6a9a774848457e690ba915b8ca38ed7c8c7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 5 Nov 2018 19:58:38 +0000 Subject: [PATCH 06/14] Clean up tests --- .../react-reconciler/src/__tests__/ReactLazy-test.internal.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index d503cbefaeac4..423fb19c346dc 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -283,7 +283,6 @@ describe('ReactLazy', () => { }); it('sets defaultProps for modern lifecycles', async () => { - const log = []; class C extends React.Component { static defaultProps = {text: 'A'}; state = {}; @@ -326,7 +325,6 @@ describe('ReactLazy', () => { } getSnapshotBeforeUpdate(prevProps) { - console.log(this.props); ReactTestRenderer.unstable_yield( `getSnapshotBeforeUpdate: ${prevProps.text} -> ${this.props.text}`, ); @@ -378,7 +376,6 @@ describe('ReactLazy', () => { }); it('sets defaultProps for legacy lifecycles', async () => { - const log = []; class C extends React.Component { static defaultProps = {text: 'A'}; state = {}; From 79151fb8b9e104ac3752392310252e6a40912a7c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 6 Nov 2018 16:21:36 +0000 Subject: [PATCH 07/14] Add another update to both tests --- .../src/__tests__/ReactLazy-test.internal.js | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 423fb19c346dc..afe24fe794cbf 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -364,7 +364,6 @@ describe('ReactLazy', () => { , ); - expect(root).toFlushAndYield([ 'getDerivedStateFromProps: A', 'shouldComponentUpdate: A -> A', @@ -373,6 +372,20 @@ describe('ReactLazy', () => { 'componentDidUpdate: A -> A', ]); expect(root).toMatchRenderedOutput('A2'); + + root.update( + }> + + , + ); + expect(root).toFlushAndYield([ + 'getDerivedStateFromProps: A', + 'shouldComponentUpdate: A -> A', + 'A3', + 'getSnapshotBeforeUpdate: A -> A', + 'componentDidUpdate: A -> A', + ]); + expect(root).toMatchRenderedOutput('A3'); }); it('sets defaultProps for legacy lifecycles', async () => { @@ -424,7 +437,6 @@ describe('ReactLazy', () => { , ); - expect(ReactTestRenderer).toHaveYielded([ 'UNSAFE_componentWillMount: A', 'A1', @@ -434,6 +446,19 @@ describe('ReactLazy', () => { ]); expect(root).toFlushAndYield([]); expect(root).toMatchRenderedOutput('A2'); + + root.update( + }> + + , + ); + expect(ReactTestRenderer).toHaveYielded([ + 'UNSAFE_componentWillReceiveProps: A -> A', + 'UNSAFE_componentWillUpdate: A -> A', + 'A3', + ]); + expect(root).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('A3'); }); it('includes lazy-loaded component in warning stack', async () => { From 981d4dd35ec08f1182a5617fbd85433584ca4754 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 5 Nov 2018 20:00:18 +0000 Subject: [PATCH 08/14] Resolve props for commit phase lifecycles --- .../src/ReactFiberBeginWork.js | 20 ++++--------------- .../src/ReactFiberCommitWork.js | 16 ++++++++++++--- .../src/ReactFiberLazyComponent.js | 15 ++++++++++++++ 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 67808381bf06a..5849d84ff1f84 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -103,7 +103,10 @@ import { resumeMountClassInstance, updateClassInstance, } from './ReactFiberClassComponent'; -import {readLazyComponentType} from './ReactFiberLazyComponent'; +import { + readLazyComponentType, + resolveDefaultProps, +} from './ReactFiberLazyComponent'; import { resolveLazyComponentTag, createFiberFromTypeAndProps, @@ -729,21 +732,6 @@ function updateHostText(current, workInProgress) { return null; } -function resolveDefaultProps(Component, baseProps) { - if (Component && Component.defaultProps) { - // Resolve default props. Taken from ReactElement - const props = Object.assign({}, baseProps); - const defaultProps = Component.defaultProps; - for (let propName in defaultProps) { - if (props[propName] === undefined) { - props[propName] = defaultProps[propName]; - } - } - return props; - } - return baseProps; -} - function mountLazyComponent( _current, workInProgress, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 138267f3fdd91..c1a6b5f68cb5a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -60,6 +60,7 @@ import {onCommitUnmount} from './ReactFiberDevToolsHook'; import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf'; import {getStackByFiberInDevAndProd} from './ReactCurrentFiber'; import {logCapturedError} from './ReactFiberErrorLogger'; +import {resolveDefaultProps} from './ReactFiberLazyComponent'; import {getCommitTime} from './ReactProfilerTimer'; import {commitUpdateQueue} from './ReactUpdateQueue'; import { @@ -345,15 +346,24 @@ function commitLifeCycles( if (finishedWork.effectTag & Update) { if (current === null) { startPhaseTimer(finishedWork, 'componentDidMount'); - instance.props = finishedWork.memoizedProps; + instance.props = resolveDefaultProps( + finishedWork.type, + finishedWork.memoizedProps, + ); instance.state = finishedWork.memoizedState; instance.componentDidMount(); stopPhaseTimer(); } else { - const prevProps = current.memoizedProps; + const prevProps = resolveDefaultProps( + finishedWork.type, + current.memoizedProps, + ); const prevState = current.memoizedState; startPhaseTimer(finishedWork, 'componentDidUpdate'); - instance.props = finishedWork.memoizedProps; + instance.props = resolveDefaultProps( + finishedWork.type, + finishedWork.memoizedProps, + ); instance.state = finishedWork.memoizedState; instance.componentDidUpdate( prevProps, diff --git a/packages/react-reconciler/src/ReactFiberLazyComponent.js b/packages/react-reconciler/src/ReactFiberLazyComponent.js index 8a002946ac076..d66de972362b0 100644 --- a/packages/react-reconciler/src/ReactFiberLazyComponent.js +++ b/packages/react-reconciler/src/ReactFiberLazyComponent.js @@ -12,6 +12,21 @@ import type {LazyComponent, Thenable} from 'shared/ReactLazyComponent'; import {Resolved, Rejected, Pending} from 'shared/ReactLazyComponent'; import warning from 'shared/warning'; +export function resolveDefaultProps(Component, baseProps) { + if (Component && Component.defaultProps) { + // Resolve default props. Taken from ReactElement + const props = Object.assign({}, baseProps); + const defaultProps = Component.defaultProps; + for (let propName in defaultProps) { + if (props[propName] === undefined) { + props[propName] = defaultProps[propName]; + } + } + return props; + } + return baseProps; +} + export function readLazyComponentType(lazyComponent: LazyComponent): T { const status = lazyComponent._status; const result = lazyComponent._result; From b3db02da9c883685714102c375e6a01f171388b9 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 5 Nov 2018 20:07:39 +0000 Subject: [PATCH 09/14] Resolve prevProps for begin phase lifecycles --- packages/react-reconciler/src/ReactFiberClassComponent.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 9a167e6f4c8ca..fc4aaec29725b 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -28,6 +28,7 @@ import warningWithoutStack from 'shared/warningWithoutStack'; import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf'; +import {resolveDefaultProps} from './ReactFiberLazyComponent'; import {StrictMode} from './ReactTypeOfMode'; import { @@ -987,7 +988,7 @@ function updateClassInstance( const instance = workInProgress.stateNode; const oldProps = workInProgress.memoizedProps; - instance.props = oldProps; + instance.props = resolveDefaultProps(workInProgress.type, oldProps); const oldContext = instance.context; const contextType = ctor.contextType; From fb4a0c9f09bee405a9417944b9c7e4d10e95c948 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 5 Nov 2018 20:09:11 +0000 Subject: [PATCH 10/14] Resolve prevProps for pre-commit lifecycles --- packages/react-reconciler/src/ReactFiberCommitWork.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index c1a6b5f68cb5a..59f40db7e647c 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -229,10 +229,13 @@ function commitBeforeMutationLifeCycles( const prevState = current.memoizedState; startPhaseTimer(finishedWork, 'getSnapshotBeforeUpdate'); const instance = finishedWork.stateNode; - instance.props = finishedWork.memoizedProps; + instance.props = resolveDefaultProps( + finishedWork.type, + finishedWork.memoizedProps, + ); instance.state = finishedWork.memoizedState; const snapshot = instance.getSnapshotBeforeUpdate( - prevProps, + resolveDefaultProps(finishedWork.type, prevProps), prevState, ); if (__DEV__) { From a4acbf58537b7d49d71af9d87f172bea8bf1da25 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 6 Nov 2018 15:46:21 +0000 Subject: [PATCH 11/14] Only resolve props if element type differs --- .../src/ReactFiberClassComponent.js | 5 ++- .../src/ReactFiberCommitWork.js | 45 ++++++++++++------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index fc4aaec29725b..abfa43e1ea552 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -988,7 +988,10 @@ function updateClassInstance( const instance = workInProgress.stateNode; const oldProps = workInProgress.memoizedProps; - instance.props = resolveDefaultProps(workInProgress.type, oldProps); + instance.props = + workInProgress.type === workInProgress.elementType + ? oldProps + : resolveDefaultProps(workInProgress.type, oldProps); const oldContext = instance.context; const contextType = ctor.contextType; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 59f40db7e647c..6f36785849afa 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -229,13 +229,18 @@ function commitBeforeMutationLifeCycles( const prevState = current.memoizedState; startPhaseTimer(finishedWork, 'getSnapshotBeforeUpdate'); const instance = finishedWork.stateNode; - instance.props = resolveDefaultProps( - finishedWork.type, - finishedWork.memoizedProps, - ); + instance.props = + finishedWork.elementType === finishedWork.type + ? finishedWork.memoizedProps + : resolveDefaultProps( + finishedWork.type, + finishedWork.memoizedProps, + ); instance.state = finishedWork.memoizedState; const snapshot = instance.getSnapshotBeforeUpdate( - resolveDefaultProps(finishedWork.type, prevProps), + finishedWork.elementType === finishedWork.type + ? prevProps + : resolveDefaultProps(finishedWork.type, prevProps), prevState, ); if (__DEV__) { @@ -349,24 +354,30 @@ function commitLifeCycles( if (finishedWork.effectTag & Update) { if (current === null) { startPhaseTimer(finishedWork, 'componentDidMount'); - instance.props = resolveDefaultProps( - finishedWork.type, - finishedWork.memoizedProps, - ); + instance.props = + finishedWork.elementType === finishedWork.type + ? finishedWork.memoizedProps + : resolveDefaultProps( + finishedWork.type, + finishedWork.memoizedProps, + ); instance.state = finishedWork.memoizedState; instance.componentDidMount(); stopPhaseTimer(); } else { - const prevProps = resolveDefaultProps( - finishedWork.type, - current.memoizedProps, - ); + const prevProps = + finishedWork.elementType === finishedWork.type + ? current.memoizedProps + : resolveDefaultProps(finishedWork.type, current.memoizedProps); const prevState = current.memoizedState; startPhaseTimer(finishedWork, 'componentDidUpdate'); - instance.props = resolveDefaultProps( - finishedWork.type, - finishedWork.memoizedProps, - ); + instance.props = + finishedWork.elementType === finishedWork.type + ? finishedWork.memoizedProps + : resolveDefaultProps( + finishedWork.type, + finishedWork.memoizedProps, + ); instance.state = finishedWork.memoizedState; instance.componentDidUpdate( prevProps, From 344eb93cae2dd5a8e8c314d24ae7d09cd5433032 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 6 Nov 2018 15:52:13 +0000 Subject: [PATCH 12/14] Fix Flow --- packages/react-reconciler/src/ReactFiberLazyComponent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberLazyComponent.js b/packages/react-reconciler/src/ReactFiberLazyComponent.js index d66de972362b0..cae98e4a1f599 100644 --- a/packages/react-reconciler/src/ReactFiberLazyComponent.js +++ b/packages/react-reconciler/src/ReactFiberLazyComponent.js @@ -12,7 +12,7 @@ import type {LazyComponent, Thenable} from 'shared/ReactLazyComponent'; import {Resolved, Rejected, Pending} from 'shared/ReactLazyComponent'; import warning from 'shared/warning'; -export function resolveDefaultProps(Component, baseProps) { +export function resolveDefaultProps(Component: any, baseProps: Object): Object { if (Component && Component.defaultProps) { // Resolve default props. Taken from ReactElement const props = Object.assign({}, baseProps); From 04417df84754a71c43a30e5e8361ef9ed8320e9a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 6 Nov 2018 16:41:54 +0000 Subject: [PATCH 13/14] Don't set instance.props/state during commit phase This is an optimization. I'm not sure it's entirely safe. It's probably worth running internal tests and see if we can ever trigger a case where they're different. This can mess with resuming. --- .../src/ReactFiberCommitWork.js | 43 +++++++------------ 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 6f36785849afa..cde0449db3dc3 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -143,8 +143,9 @@ export function logError(boundary: Fiber, errorInfo: CapturedValue) { const callComponentWillUnmountWithTimer = function(current, instance) { startPhaseTimer(current, 'componentWillUnmount'); - instance.props = current.memoizedProps; - instance.state = current.memoizedState; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. instance.componentWillUnmount(); stopPhaseTimer(); }; @@ -229,14 +230,9 @@ function commitBeforeMutationLifeCycles( const prevState = current.memoizedState; startPhaseTimer(finishedWork, 'getSnapshotBeforeUpdate'); const instance = finishedWork.stateNode; - instance.props = - finishedWork.elementType === finishedWork.type - ? finishedWork.memoizedProps - : resolveDefaultProps( - finishedWork.type, - finishedWork.memoizedProps, - ); - instance.state = finishedWork.memoizedState; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. const snapshot = instance.getSnapshotBeforeUpdate( finishedWork.elementType === finishedWork.type ? prevProps @@ -354,14 +350,9 @@ function commitLifeCycles( if (finishedWork.effectTag & Update) { if (current === null) { startPhaseTimer(finishedWork, 'componentDidMount'); - instance.props = - finishedWork.elementType === finishedWork.type - ? finishedWork.memoizedProps - : resolveDefaultProps( - finishedWork.type, - finishedWork.memoizedProps, - ); - instance.state = finishedWork.memoizedState; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. instance.componentDidMount(); stopPhaseTimer(); } else { @@ -371,14 +362,9 @@ function commitLifeCycles( : resolveDefaultProps(finishedWork.type, current.memoizedProps); const prevState = current.memoizedState; startPhaseTimer(finishedWork, 'componentDidUpdate'); - instance.props = - finishedWork.elementType === finishedWork.type - ? finishedWork.memoizedProps - : resolveDefaultProps( - finishedWork.type, - finishedWork.memoizedProps, - ); - instance.state = finishedWork.memoizedState; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. instance.componentDidUpdate( prevProps, prevState, @@ -389,8 +375,9 @@ function commitLifeCycles( } const updateQueue = finishedWork.updateQueue; if (updateQueue !== null) { - instance.props = finishedWork.memoizedProps; - instance.state = finishedWork.memoizedState; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. commitUpdateQueue( finishedWork, updateQueue, From 40aeb2e87b0f5cd924a7952a5722cf127bdfee4b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 6 Nov 2018 16:43:50 +0000 Subject: [PATCH 14/14] Keep setting instance.props/state before unmounting This reverts part of the previous commit. It broke a test that verifies we use current props in componentWillUnmount if the fiber unmounts due to an error. --- packages/react-reconciler/src/ReactFiberCommitWork.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index cde0449db3dc3..9a0ef664b68fd 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -143,9 +143,8 @@ export function logError(boundary: Fiber, errorInfo: CapturedValue) { const callComponentWillUnmountWithTimer = function(current, instance) { startPhaseTimer(current, 'componentWillUnmount'); - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. + instance.props = current.memoizedProps; + instance.state = current.memoizedState; instance.componentWillUnmount(); stopPhaseTimer(); };