From 4cf72db327109ca0c8efb29fc80ddaad2ff60e8f Mon Sep 17 00:00:00 2001 From: jddxf <740531372@qq.com> Date: Sat, 4 Apr 2020 23:58:34 +0800 Subject: [PATCH 1/2] Add failing tests for lazy components --- .../src/__tests__/ReactLazy-test.internal.js | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index d542173fce30..33533a754586 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -343,6 +343,97 @@ describe('ReactLazy', () => { expect(root).toMatchRenderedOutput('SiblingB'); }); + it('resolves defaultProps without breaking bailout due to unchanged props and state, #17151', async () => { + class LazyImpl extends React.Component { + static defaultProps = {value: 0}; + + render() { + const text = `${this.props.label}: ${this.props.value}`; + return ; + } + } + + const Lazy = lazy(() => fakeImport(LazyImpl)); + + const instance1 = React.createRef(null); + const instance2 = React.createRef(null); + + const root = ReactTestRenderer.create( + <> + + }> + + + , + { + unstable_isConcurrent: true, + }, + ); + expect(Scheduler).toFlushAndYield(['Not lazy: 0', 'Loading...']); + expect(root).not.toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + + await Promise.resolve(); + + expect(Scheduler).toFlushAndYield(['Lazy: 0']); + expect(root).toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + + // Should bailout due to unchanged props and state + instance1.current.setState(null); + expect(Scheduler).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + + // Should bailout due to unchanged props and state + instance2.current.setState(null); + expect(Scheduler).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + }); + + it('resolves defaultProps without breaking bailout in PureComponent, #17151', async () => { + class LazyImpl extends React.PureComponent { + static defaultProps = {value: 0}; + state = {}; + + render() { + const text = `${this.props.label}: ${this.props.value}`; + return ; + } + } + + const Lazy = lazy(() => fakeImport(LazyImpl)); + + const instance1 = React.createRef(null); + const instance2 = React.createRef(null); + + const root = ReactTestRenderer.create( + <> + + }> + + + , + { + unstable_isConcurrent: true, + }, + ); + expect(Scheduler).toFlushAndYield(['Not lazy: 0', 'Loading...']); + expect(root).not.toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + + await Promise.resolve(); + + expect(Scheduler).toFlushAndYield(['Lazy: 0']); + expect(root).toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + + // Should bailout due to shallow equal props and state + instance1.current.setState({}); + expect(Scheduler).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + + // Should bailout due to shallow equal props and state + instance2.current.setState({}); + expect(Scheduler).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('Not lazy: 0Lazy: 0'); + }); + it('sets defaultProps for modern lifecycles', async () => { class C extends React.Component { static defaultProps = {text: 'A'}; From 92c15f27385e9ec2c39c9d4deb90e52527df55ef Mon Sep 17 00:00:00 2001 From: jddxf <740531372@qq.com> Date: Sun, 5 Apr 2020 00:41:51 +0800 Subject: [PATCH 2/2] Fix bailout broken in lazy components due to default props resolving We should never compare unresolved props with resolved props. Since comparing resolved props by reference doesn't make sense, we use unresolved props in that case. Otherwise, resolved props are used. --- .../src/ReactFiberClassComponent.js | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 7b9be3544e71..dce415db1a24 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -997,11 +997,13 @@ function updateClassInstance( cloneUpdateQueue(current, workInProgress); - const oldProps = workInProgress.memoizedProps; - instance.props = + const unresolvedOldProps = workInProgress.memoizedProps; + const oldProps = workInProgress.type === workInProgress.elementType - ? oldProps - : resolveDefaultProps(workInProgress.type, oldProps); + ? unresolvedOldProps + : resolveDefaultProps(workInProgress.type, unresolvedOldProps); + instance.props = oldProps; + const unresolvedNewProps = workInProgress.pendingProps; const oldContext = instance.context; const contextType = ctor.contextType; @@ -1029,7 +1031,10 @@ function updateClassInstance( (typeof instance.UNSAFE_componentWillReceiveProps === 'function' || typeof instance.componentWillReceiveProps === 'function') ) { - if (oldProps !== newProps || oldContext !== nextContext) { + if ( + unresolvedOldProps !== unresolvedNewProps || + oldContext !== nextContext + ) { callComponentWillReceiveProps( workInProgress, instance, @@ -1047,7 +1052,7 @@ function updateClassInstance( newState = workInProgress.memoizedState; if ( - oldProps === newProps && + unresolvedOldProps === unresolvedNewProps && oldState === newState && !hasContextChanged() && !checkHasForceUpdateAfterProcessing() @@ -1056,7 +1061,7 @@ function updateClassInstance( // effect even though we're bailing out, so that cWU/cDU are called. if (typeof instance.componentDidUpdate === 'function') { if ( - oldProps !== current.memoizedProps || + unresolvedOldProps !== current.memoizedProps || oldState !== current.memoizedState ) { workInProgress.effectTag |= Update; @@ -1064,12 +1069,13 @@ function updateClassInstance( } if (typeof instance.getSnapshotBeforeUpdate === 'function') { if ( - oldProps !== current.memoizedProps || + unresolvedOldProps !== current.memoizedProps || oldState !== current.memoizedState ) { workInProgress.effectTag |= Snapshot; } } + instance.props = workInProgress.memoizedProps = newProps; return false; } @@ -1121,7 +1127,7 @@ function updateClassInstance( // effect even though we're bailing out, so that cWU/cDU are called. if (typeof instance.componentDidUpdate === 'function') { if ( - oldProps !== current.memoizedProps || + unresolvedOldProps !== current.memoizedProps || oldState !== current.memoizedState ) { workInProgress.effectTag |= Update; @@ -1129,7 +1135,7 @@ function updateClassInstance( } if (typeof instance.getSnapshotBeforeUpdate === 'function') { if ( - oldProps !== current.memoizedProps || + unresolvedOldProps !== current.memoizedProps || oldState !== current.memoizedState ) { workInProgress.effectTag |= Snapshot;