From ffe59a8100de66eae3c0c46be23ffc83f88a1b28 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Nov 2020 19:36:41 -0500 Subject: [PATCH 1/5] Reconcile element types of lazy component yielding the same type --- .../src/ReactChildFiber.new.js | 102 +++++++++--------- .../src/ReactChildFiber.old.js | 102 +++++++++--------- .../src/__tests__/ReactLazy-test.internal.js | 87 +++++++++++++++ 3 files changed, 193 insertions(+), 98 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index 601f3b21e4395..b99876fa4a155 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -246,6 +246,12 @@ function warnOnFunctionType(returnFiber: Fiber) { } } +function resolveLazy(lazyType) { + const payload = lazyType._payload; + const init = lazyType._init; + return init(payload); +} + // This wrapper function exists because I expect to clone the code in each path // to be able to optimize each path individually by branching early. This needs // a compiler or we can do it manually. Helpers that don't need this branching @@ -383,9 +389,24 @@ function ChildReconciler(shouldTrackSideEffects) { element: ReactElement, lanes: Lanes, ): Fiber { + const elementType = element.type; + if (elementType === REACT_FRAGMENT_TYPE) { + return updateFragment( + returnFiber, + current, + element.props.children, + lanes, + element.key, + ); + } if (current !== null) { if ( - current.elementType === element.type || + current.elementType === elementType || + (enableLazyElements && + typeof elementType === 'object' && + elementType !== null && + elementType.$$typeof === REACT_LAZY_TYPE && + resolveLazy(elementType) === current.type) || // Keep this check inline so it only runs on the false path: (__DEV__ ? isCompatibleFamilyForHotReloading(current, element) : false) ) { @@ -551,15 +572,6 @@ function ChildReconciler(shouldTrackSideEffects) { switch (newChild.$$typeof) { case REACT_ELEMENT_TYPE: { if (newChild.key === key) { - if (newChild.type === REACT_FRAGMENT_TYPE) { - return updateFragment( - returnFiber, - oldFiber, - newChild.props.children, - lanes, - key, - ); - } return updateElement(returnFiber, oldFiber, newChild, lanes); } else { return null; @@ -622,15 +634,6 @@ function ChildReconciler(shouldTrackSideEffects) { existingChildren.get( newChild.key === null ? newIdx : newChild.key, ) || null; - if (newChild.type === REACT_FRAGMENT_TYPE) { - return updateFragment( - returnFiber, - matchedFiber, - newChild.props.children, - lanes, - newChild.key, - ); - } return updateElement(returnFiber, matchedFiber, newChild, lanes); } case REACT_PORTAL_TYPE: { @@ -1101,39 +1104,40 @@ function ChildReconciler(shouldTrackSideEffects) { // TODO: If key === null and child.key === null, then this only applies to // the first item in the list. if (child.key === key) { - switch (child.tag) { - case Fragment: { - if (element.type === REACT_FRAGMENT_TYPE) { - deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, element.props.children); - existing.return = returnFiber; - if (__DEV__) { - existing._debugSource = element._source; - existing._debugOwner = element._owner; - } - return existing; + const elementType = element.type; + if (elementType === REACT_FRAGMENT_TYPE) { + if (child.tag === Fragment) { + deleteRemainingChildren(returnFiber, child.sibling); + const existing = useFiber(child, element.props.children); + existing.return = returnFiber; + if (__DEV__) { + existing._debugSource = element._source; + existing._debugOwner = element._owner; } - break; + return existing; } - default: { - if ( - child.elementType === element.type || - // Keep this check inline so it only runs on the false path: - (__DEV__ - ? isCompatibleFamilyForHotReloading(child, element) - : false) - ) { - deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, element.props); - existing.ref = coerceRef(returnFiber, child, element); - existing.return = returnFiber; - if (__DEV__) { - existing._debugSource = element._source; - existing._debugOwner = element._owner; - } - return existing; + } else { + if ( + child.elementType === elementType || + (enableLazyElements && + typeof elementType === 'object' && + elementType !== null && + elementType.$$typeof === REACT_LAZY_TYPE && + resolveLazy(elementType) === child.type) || + // Keep this check inline so it only runs on the false path: + (__DEV__ + ? isCompatibleFamilyForHotReloading(child, element) + : false) + ) { + deleteRemainingChildren(returnFiber, child.sibling); + const existing = useFiber(child, element.props); + existing.ref = coerceRef(returnFiber, child, element); + existing.return = returnFiber; + if (__DEV__) { + existing._debugSource = element._source; + existing._debugOwner = element._owner; } - break; + return existing; } } // Didn't match. diff --git a/packages/react-reconciler/src/ReactChildFiber.old.js b/packages/react-reconciler/src/ReactChildFiber.old.js index adb9c0418df0e..cde52aef0f6ab 100644 --- a/packages/react-reconciler/src/ReactChildFiber.old.js +++ b/packages/react-reconciler/src/ReactChildFiber.old.js @@ -246,6 +246,12 @@ function warnOnFunctionType(returnFiber: Fiber) { } } +function resolveLazy(lazyType) { + const payload = lazyType._payload; + const init = lazyType._init; + return init(payload); +} + // This wrapper function exists because I expect to clone the code in each path // to be able to optimize each path individually by branching early. This needs // a compiler or we can do it manually. Helpers that don't need this branching @@ -383,9 +389,24 @@ function ChildReconciler(shouldTrackSideEffects) { element: ReactElement, lanes: Lanes, ): Fiber { + const elementType = element.type; + if (elementType === REACT_FRAGMENT_TYPE) { + return updateFragment( + returnFiber, + current, + element.props.children, + lanes, + element.key, + ); + } if (current !== null) { if ( - current.elementType === element.type || + current.elementType === elementType || + (enableLazyElements && + typeof elementType === 'object' && + elementType !== null && + elementType.$$typeof === REACT_LAZY_TYPE && + resolveLazy(elementType) === current.type) || // Keep this check inline so it only runs on the false path: (__DEV__ ? isCompatibleFamilyForHotReloading(current, element) : false) ) { @@ -551,15 +572,6 @@ function ChildReconciler(shouldTrackSideEffects) { switch (newChild.$$typeof) { case REACT_ELEMENT_TYPE: { if (newChild.key === key) { - if (newChild.type === REACT_FRAGMENT_TYPE) { - return updateFragment( - returnFiber, - oldFiber, - newChild.props.children, - lanes, - key, - ); - } return updateElement(returnFiber, oldFiber, newChild, lanes); } else { return null; @@ -622,15 +634,6 @@ function ChildReconciler(shouldTrackSideEffects) { existingChildren.get( newChild.key === null ? newIdx : newChild.key, ) || null; - if (newChild.type === REACT_FRAGMENT_TYPE) { - return updateFragment( - returnFiber, - matchedFiber, - newChild.props.children, - lanes, - newChild.key, - ); - } return updateElement(returnFiber, matchedFiber, newChild, lanes); } case REACT_PORTAL_TYPE: { @@ -1101,39 +1104,40 @@ function ChildReconciler(shouldTrackSideEffects) { // TODO: If key === null and child.key === null, then this only applies to // the first item in the list. if (child.key === key) { - switch (child.tag) { - case Fragment: { - if (element.type === REACT_FRAGMENT_TYPE) { - deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, element.props.children); - existing.return = returnFiber; - if (__DEV__) { - existing._debugSource = element._source; - existing._debugOwner = element._owner; - } - return existing; + const elementType = element.type; + if (elementType === REACT_FRAGMENT_TYPE) { + if (child.tag === Fragment) { + deleteRemainingChildren(returnFiber, child.sibling); + const existing = useFiber(child, element.props.children); + existing.return = returnFiber; + if (__DEV__) { + existing._debugSource = element._source; + existing._debugOwner = element._owner; } - break; + return existing; } - default: { - if ( - child.elementType === element.type || - // Keep this check inline so it only runs on the false path: - (__DEV__ - ? isCompatibleFamilyForHotReloading(child, element) - : false) - ) { - deleteRemainingChildren(returnFiber, child.sibling); - const existing = useFiber(child, element.props); - existing.ref = coerceRef(returnFiber, child, element); - existing.return = returnFiber; - if (__DEV__) { - existing._debugSource = element._source; - existing._debugOwner = element._owner; - } - return existing; + } else { + if ( + child.elementType === elementType || + (enableLazyElements && + typeof elementType === 'object' && + elementType !== null && + elementType.$$typeof === REACT_LAZY_TYPE && + resolveLazy(elementType) === child.type) || + // Keep this check inline so it only runs on the false path: + (__DEV__ + ? isCompatibleFamilyForHotReloading(child, element) + : false) + ) { + deleteRemainingChildren(returnFiber, child.sibling); + const existing = useFiber(child, element.props); + existing.ref = coerceRef(returnFiber, child, element); + existing.return = returnFiber; + if (__DEV__) { + existing._debugSource = element._source; + existing._debugOwner = element._owner; } - break; + return existing; } } // Didn't match. diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 55c15e7412eab..80ab9b132de27 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -1268,6 +1268,93 @@ describe('ReactLazy', () => { expect(componentStackMessage).toContain('in Lazy'); }); + // @gate enableLazyElements + it('mount and reorder lazy types', async () => { + class Child extends React.Component { + componentDidMount() { + Scheduler.unstable_yieldValue('Did mount: ' + this.props.label); + } + componentDidUpdate() { + Scheduler.unstable_yieldValue('Did update: ' + this.props.label); + } + render() { + return ; + } + } + + function ChildA({lowerCase}) { + return ; + } + + function ChildB({lowerCase}) { + return ; + } + + const LazyChildA = lazy(() => { + Scheduler.unstable_yieldValue('Init A'); + return fakeImport(ChildA); + }); + const LazyChildB = lazy(() => { + Scheduler.unstable_yieldValue('Init B'); + return fakeImport(ChildB); + }); + const LazyChildA2 = lazy(() => { + Scheduler.unstable_yieldValue('Init A2'); + return fakeImport(ChildA); + }); + const LazyChildB2 = lazy(() => { + Scheduler.unstable_yieldValue('Init B2'); + return fakeImport(ChildB); + }); + + function Parent({swap}) { + return ( + }> + {swap + ? [ + , + , + ] + : [, ]} + + ); + } + + const root = ReactTestRenderer.create(, { + unstable_isConcurrent: true, + }); + + expect(Scheduler).toFlushAndYield(['Init A', 'Init B', 'Loading...']); + expect(root).not.toMatchRenderedOutput('AB'); + + await LazyChildA; + await LazyChildB; + + expect(Scheduler).toFlushAndYield([ + 'A', + 'B', + 'Did mount: A', + 'Did mount: B', + ]); + expect(root).toMatchRenderedOutput('AB'); + + // Swap the position of A and B + root.update(); + expect(Scheduler).toFlushAndYield(['Init B2', 'Loading...']); + await LazyChildB2; + // We need to flush to trigger the second one to load. + expect(Scheduler).toFlushAndYield(['Init A2', 'Loading...']); + await LazyChildA2; + + expect(Scheduler).toFlushAndYield([ + 'b', + 'a', + 'Did update: b', + 'Did update: a', + ]); + expect(root).toMatchRenderedOutput('ba'); + }); + // @gate enableLazyElements it('mount and reorder lazy elements', async () => { class Child extends React.Component { From 135e5c1ae552b7c8b00c4c7ebcdca1e9df289134 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Nov 2020 20:56:34 -0500 Subject: [PATCH 2/5] Add some legacy mode and suspense boundary flushing tests --- .../src/__tests__/ReactLazy-test.internal.js | 196 +++++++++++++++++- 1 file changed, 186 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 80ab9b132de27..23d333ab8ba24 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -1302,20 +1302,25 @@ describe('ReactLazy', () => { Scheduler.unstable_yieldValue('Init A2'); return fakeImport(ChildA); }); + let resolveB2; const LazyChildB2 = lazy(() => { Scheduler.unstable_yieldValue('Init B2'); - return fakeImport(ChildB); + return new Promise(r => { + resolveB2 = r; + }); }); function Parent({swap}) { return ( - }> - {swap - ? [ - , - , - ] - : [, ]} + }> + }> + {swap + ? [ + , + , + ] + : [, ]} + ); } @@ -1338,12 +1343,107 @@ describe('ReactLazy', () => { ]); expect(root).toMatchRenderedOutput('AB'); + // Swap the position of A and B + ReactTestRenderer.act(() => { + root.update(); + expect(Scheduler).toFlushAndYield(['Init B2', 'Loading...']); + }); + + // The suspense boundary should've triggered now. + expect(root).toMatchRenderedOutput('Loading...'); + await resolveB2({default: ChildB}); + + // We need to flush to trigger the second one to load. + expect(Scheduler).toFlushAndYield(['Init A2']); + await LazyChildA2; + + expect(Scheduler).toFlushAndYield([ + 'b', + 'a', + 'Did update: b', + 'Did update: a', + ]); + expect(root).toMatchRenderedOutput('ba'); + }); + + // @gate enableLazyElements + it('mount and reorder lazy types (legacy mode)', async () => { + class Child extends React.Component { + componentDidMount() { + Scheduler.unstable_yieldValue('Did mount: ' + this.props.label); + } + componentDidUpdate() { + Scheduler.unstable_yieldValue('Did update: ' + this.props.label); + } + render() { + return ; + } + } + + function ChildA({lowerCase}) { + return ; + } + + function ChildB({lowerCase}) { + return ; + } + + const LazyChildA = lazy(() => { + Scheduler.unstable_yieldValue('Init A'); + return fakeImport(ChildA); + }); + const LazyChildB = lazy(() => { + Scheduler.unstable_yieldValue('Init B'); + return fakeImport(ChildB); + }); + const LazyChildA2 = lazy(() => { + Scheduler.unstable_yieldValue('Init A2'); + return fakeImport(ChildA); + }); + const LazyChildB2 = lazy(() => { + Scheduler.unstable_yieldValue('Init B2'); + return fakeImport(ChildB); + }); + + function Parent({swap}) { + return ( + }> + }> + {swap + ? [ + , + , + ] + : [, ]} + + + ); + } + + const root = ReactTestRenderer.create(, { + unstable_isConcurrent: false, + }); + + expect(Scheduler).toHaveYielded(['Init A', 'Init B', 'Loading...']); + expect(root).not.toMatchRenderedOutput('AB'); + + await LazyChildA; + await LazyChildB; + + expect(Scheduler).toFlushAndYield([ + 'A', + 'B', + 'Did mount: A', + 'Did mount: B', + ]); + expect(root).toMatchRenderedOutput('AB'); + // Swap the position of A and B root.update(); - expect(Scheduler).toFlushAndYield(['Init B2', 'Loading...']); + expect(Scheduler).toHaveYielded(['Init B2', 'Loading...']); await LazyChildB2; // We need to flush to trigger the second one to load. - expect(Scheduler).toFlushAndYield(['Init A2', 'Loading...']); + expect(Scheduler).toFlushAndYield(['Init A2']); await LazyChildA2; expect(Scheduler).toFlushAndYield([ @@ -1430,4 +1530,80 @@ describe('ReactLazy', () => { ]); expect(root).toMatchRenderedOutput('ba'); }); + + // @gate enableLazyElements + it('mount and reorder lazy elements (legacy mode)', async () => { + class Child extends React.Component { + componentDidMount() { + Scheduler.unstable_yieldValue('Did mount: ' + this.props.label); + } + componentDidUpdate() { + Scheduler.unstable_yieldValue('Did update: ' + this.props.label); + } + render() { + return ; + } + } + + const lazyChildA = lazy(() => { + Scheduler.unstable_yieldValue('Init A'); + return fakeImport(); + }); + const lazyChildB = lazy(() => { + Scheduler.unstable_yieldValue('Init B'); + return fakeImport(); + }); + const lazyChildA2 = lazy(() => { + Scheduler.unstable_yieldValue('Init A2'); + return fakeImport(); + }); + const lazyChildB2 = lazy(() => { + Scheduler.unstable_yieldValue('Init B2'); + return fakeImport(); + }); + + function Parent({swap}) { + return ( + }> + {swap ? [lazyChildB2, lazyChildA2] : [lazyChildA, lazyChildB]} + + ); + } + + const root = ReactTestRenderer.create(, { + unstable_isConcurrent: false, + }); + + expect(Scheduler).toHaveYielded(['Init A', 'Loading...']); + expect(root).not.toMatchRenderedOutput('AB'); + + await lazyChildA; + // We need to flush to trigger the B to load. + expect(Scheduler).toFlushAndYield(['Init B']); + await lazyChildB; + + expect(Scheduler).toFlushAndYield([ + 'A', + 'B', + 'Did mount: A', + 'Did mount: B', + ]); + expect(root).toMatchRenderedOutput('AB'); + + // Swap the position of A and B + root.update(); + expect(Scheduler).toHaveYielded(['Init B2', 'Loading...']); + await lazyChildB2; + // We need to flush to trigger the second one to load. + expect(Scheduler).toFlushAndYield(['Init A2']); + await lazyChildA2; + + expect(Scheduler).toFlushAndYield([ + 'b', + 'a', + 'Did update: b', + 'Did update: a', + ]); + expect(root).toMatchRenderedOutput('ba'); + }); }); From b5012466af5c5a2607ab4b056a17d9019ce80e4c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Nov 2020 21:11:53 -0500 Subject: [PATCH 3/5] Fix infinite loop in legacy mode In legacy mode we typically commit the suspending fiber and then rerender the nearest boundary to render the fallback in a separate commit. We can't do that when the boundary itself suspends because when we try to do the second pass, it'll suspend again and infinite loop. Interestingly the legacy semantics are not needed in this case because they exist to let an existing partial render fully commit its partial state. In this case there's no partial state, so we can just render the fallback immediately instead. --- packages/react-reconciler/src/ReactFiberThrow.new.js | 10 +++++++++- packages/react-reconciler/src/ReactFiberThrow.old.js | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 9dd9df6d0cfe5..01ab69cecee66 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -256,7 +256,15 @@ function throwException( // Note: It doesn't matter whether the component that suspended was // inside a blocking mode tree. If the Suspense is outside of it, we // should *not* suspend the commit. - if ((workInProgress.mode & BlockingMode) === NoMode) { + // + // If the suspense boundary suspended itself suspended, we don't have to + // do this trick because nothing was partially started. We can just + // directly do a second pass over the fallback in this render and + // pretend we meant to render that directly. + if ( + (workInProgress.mode & BlockingMode) === NoMode && + workInProgress !== returnFiber + ) { workInProgress.flags |= DidCapture; sourceFiber.flags |= ForceUpdateForLegacySuspense; diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 8d338d552d7d6..fabcffa87378a 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -256,7 +256,15 @@ function throwException( // Note: It doesn't matter whether the component that suspended was // inside a blocking mode tree. If the Suspense is outside of it, we // should *not* suspend the commit. - if ((workInProgress.mode & BlockingMode) === NoMode) { + // + // If the suspense boundary suspended itself suspended, we don't have to + // do this trick because nothing was partially started. We can just + // directly do a second pass over the fallback in this render and + // pretend we meant to render that directly. + if ( + (workInProgress.mode & BlockingMode) === NoMode && + workInProgress !== returnFiber + ) { workInProgress.flags |= DidCapture; sourceFiber.flags |= ForceUpdateForLegacySuspense; From 76d4a7d678fb1b0e5c629532de9f508896c2f755 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Nov 2020 22:28:21 -0500 Subject: [PATCH 4/5] Check fast refresh compatibility first resolveLazy can suspend and if it does, it can resuspend. Fast refresh assumes that we don't resuspend. Instead it relies on updating the inner component later. --- .../src/ReactChildFiber.new.js | 26 +++++++++++++------ .../src/ReactChildFiber.old.js | 25 ++++++++++++------ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index b99876fa4a155..534d7032ba2d9 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -402,13 +402,19 @@ function ChildReconciler(shouldTrackSideEffects) { if (current !== null) { if ( current.elementType === elementType || + // Keep this check inline so it only runs on the false path: + (__DEV__ + ? isCompatibleFamilyForHotReloading(current, element) + : false) || + // Lazy types should reconcile their resolved type. + // We need to do this after the Hot Reloading check above, + // because hot reloading has different semantics than prod because + // it doesn't resuspend. So we can't let the call below suspend. (enableLazyElements && typeof elementType === 'object' && elementType !== null && elementType.$$typeof === REACT_LAZY_TYPE && - resolveLazy(elementType) === current.type) || - // Keep this check inline so it only runs on the false path: - (__DEV__ ? isCompatibleFamilyForHotReloading(current, element) : false) + resolveLazy(elementType) === current.type) ) { // Move based on index const existing = useFiber(current, element.props); @@ -1119,15 +1125,19 @@ function ChildReconciler(shouldTrackSideEffects) { } else { if ( child.elementType === elementType || + // Keep this check inline so it only runs on the false path: + (__DEV__ + ? isCompatibleFamilyForHotReloading(child, element) + : false) || + // Lazy types should reconcile their resolved type. + // We need to do this after the Hot Reloading check above, + // because hot reloading has different semantics than prod because + // it doesn't resuspend. So we can't let the call below suspend. (enableLazyElements && typeof elementType === 'object' && elementType !== null && elementType.$$typeof === REACT_LAZY_TYPE && - resolveLazy(elementType) === child.type) || - // Keep this check inline so it only runs on the false path: - (__DEV__ - ? isCompatibleFamilyForHotReloading(child, element) - : false) + resolveLazy(elementType) === child.type) ) { deleteRemainingChildren(returnFiber, child.sibling); const existing = useFiber(child, element.props); diff --git a/packages/react-reconciler/src/ReactChildFiber.old.js b/packages/react-reconciler/src/ReactChildFiber.old.js index cde52aef0f6ab..df4c2e10d9299 100644 --- a/packages/react-reconciler/src/ReactChildFiber.old.js +++ b/packages/react-reconciler/src/ReactChildFiber.old.js @@ -402,13 +402,19 @@ function ChildReconciler(shouldTrackSideEffects) { if (current !== null) { if ( current.elementType === elementType || + // Keep this check inline so it only runs on the false path: + (__DEV__ + ? isCompatibleFamilyForHotReloading(current, element) + : false) || + // Lazy types should reconcile their resolved type. + // We need to do this after the Hot Reloading check above, + // because hot reloading has different semantics than prod because + // it doesn't resuspend. So we can't let the call below suspend. (enableLazyElements && typeof elementType === 'object' && elementType !== null && elementType.$$typeof === REACT_LAZY_TYPE && - resolveLazy(elementType) === current.type) || - // Keep this check inline so it only runs on the false path: - (__DEV__ ? isCompatibleFamilyForHotReloading(current, element) : false) + resolveLazy(elementType) === current.type) ) { // Move based on index const existing = useFiber(current, element.props); @@ -1119,15 +1125,18 @@ function ChildReconciler(shouldTrackSideEffects) { } else { if ( child.elementType === elementType || + (__DEV__ + ? isCompatibleFamilyForHotReloading(child, element) + : false) || + // Lazy types should reconcile their resolved type. + // We need to do this after the Hot Reloading check above, + // because hot reloading has different semantics than prod because + // it doesn't resuspend. So we can't let the call below suspend. (enableLazyElements && typeof elementType === 'object' && elementType !== null && elementType.$$typeof === REACT_LAZY_TYPE && - resolveLazy(elementType) === child.type) || - // Keep this check inline so it only runs on the false path: - (__DEV__ - ? isCompatibleFamilyForHotReloading(child, element) - : false) + resolveLazy(elementType) === child.type) ) { deleteRemainingChildren(returnFiber, child.sibling); const existing = useFiber(child, element.props); From 9fb5014dec48cca827464e8700010a06142b7e17 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Nov 2020 22:41:37 -0500 Subject: [PATCH 5/5] Use timers instead of act to force fallbacks to show --- .../src/__tests__/ReactLazy-test.internal.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 23d333ab8ba24..b7f1570823c14 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -1344,10 +1344,9 @@ describe('ReactLazy', () => { expect(root).toMatchRenderedOutput('AB'); // Swap the position of A and B - ReactTestRenderer.act(() => { - root.update(); - expect(Scheduler).toFlushAndYield(['Init B2', 'Loading...']); - }); + root.update(); + expect(Scheduler).toFlushAndYield(['Init B2', 'Loading...']); + jest.runAllTimers(); // The suspense boundary should've triggered now. expect(root).toMatchRenderedOutput('Loading...');