From 01c1093a390d790fa145d83c9049009498808d01 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 8 Nov 2018 01:54:21 -0800 Subject: [PATCH] Moar fuzziness Adds more fuzziness to the generated tests. Specifcally, introduces nested Suspense cases, where the fallback of a Suspense component also suspends. This flushed out a bug (yay!) whose test case I've hard coded. --- .../src/ReactFiberBeginWork.js | 9 ++-- .../src/ReactFiberCommitWork.js | 10 +++++ .../src/ReactFiberCompleteWork.js | 18 +++++--- .../src/ReactFiberUnwindWork.js | 9 ---- .../ReactSuspenseFuzz-test.internal.js | 45 ++++++++++++++++++- ...tSuspenseWithNoopRenderer-test.internal.js | 3 ++ 6 files changed, 73 insertions(+), 21 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 080f3a1fa8361..902e424c3505b 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -1178,7 +1178,6 @@ function updateSuspenseComponent( currentPrimaryChildFragment.pendingProps, NoWork, ); - primaryChildFragment.effectTag |= Placement; if ((workInProgress.mode & ConcurrentMode) === NoContext) { // Outside of concurrent mode, we commit the effects from the @@ -1213,7 +1212,6 @@ function updateSuspenseComponent( nextFallbackChildren, currentFallbackChildFragment.expirationTime, )); - fallbackChildFragment.effectTag |= Placement; child = primaryChildFragment; primaryChildFragment.childExpirationTime = NoWork; // Skip the primary children, and continue working on the @@ -1257,11 +1255,14 @@ function updateSuspenseComponent( NoWork, null, ); - - primaryChildFragment.effectTag |= Placement; primaryChildFragment.child = currentPrimaryChild; currentPrimaryChild.return = primaryChildFragment; + // Even though we're creating a new fiber, there are no new children, + // because we're reusing an already mounted tree. So we don't need to + // schedule a placement. + // primaryChildFragment.effectTag |= Placement; + if ((workInProgress.mode & ConcurrentMode) === NoContext) { // Outside of concurrent mode, we commit the effects from the // partially completed, timed-out tree, too. diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index f22e3efb5d717..55a39ee640db8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -558,6 +558,16 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) { } else { unhideTextInstance(instance, node.memoizedProps); } + } else if ( + node.tag === SuspenseComponent && + node.memoizedState !== null + ) { + // Found a nested Suspense component that timed out. Skip over the + // primary child fragment, which should remain hidden. + const fallbackChildFragment: Fiber = (node.child: any).sibling; + fallbackChildFragment.return = node; + node = fallbackChildFragment; + continue; } else if (node.child !== null) { node.child.return = node; node = node.child; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 24126f026cc49..fbd3b5b65fffc 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -45,6 +45,7 @@ import { Update, NoEffect, DidCapture, + Deletion, } from 'shared/ReactSideEffectTags'; import invariant from 'shared/invariant'; @@ -82,7 +83,6 @@ import { popHydrationState, } from './ReactFiberHydrationContext'; import {ConcurrentMode, NoContext} from './ReactTypeOfMode'; -import {reconcileChildFibers} from './ReactChildFiber'; function markUpdate(workInProgress: Fiber) { // Tag the fiber with an update effect. This turns a Placement into @@ -715,12 +715,16 @@ function completeWork( // the stateNode during the begin phase? const currentFallbackChild: Fiber | null = (current.child: any).sibling; if (currentFallbackChild !== null) { - reconcileChildFibers( - workInProgress, - currentFallbackChild, - null, - renderExpirationTime, - ); + // Deletions go at the beginning of the return fiber's effect list + const first = workInProgress.firstEffect; + if (first !== null) { + workInProgress.firstEffect = currentFallbackChild; + currentFallbackChild.nextEffect = first; + } else { + workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChild; + currentFallbackChild.nextEffect = null; + } + currentFallbackChild.effectTag = Deletion; } } diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index 6fb2e080687e5..4160e6d8fdd5c 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -70,7 +70,6 @@ import { LOW_PRIORITY_EXPIRATION, } from './ReactFiberExpirationTime'; import {findEarliestOutstandingPriorityLevel} from './ReactFiberPendingPriority'; -import {reconcileChildren} from './ReactFiberBeginWork'; function createRootErrorUpdate( fiber: Fiber, @@ -238,14 +237,6 @@ function throwException( if ((workInProgress.mode & ConcurrentMode) === NoEffect) { workInProgress.effectTag |= DidCapture; - // Unmount the source fiber's children - const nextChildren = null; - reconcileChildren( - sourceFiber.alternate, - sourceFiber, - nextChildren, - renderExpirationTime, - ); sourceFiber.effectTag &= ~Incomplete; // We're going to commit this fiber even though it didn't complete. diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js index be2afc82f8274..540bc02e178e0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js @@ -296,7 +296,28 @@ describe('ReactSuspenseFuzz', () => { {value: randomInteger(0, 5000), weight: 1}, ]); - return React.createElement(Suspense, {maxDuration}, ...children); + const fallbackType = pickRandomWeighted([ + {value: 'none', weight: 1}, + {value: 'normal', weight: 1}, + {value: 'nested suspense', weight: 1}, + ]); + + let fallback; + if (fallbackType === 'normal') { + fallback = 'Loading...'; + } else if (fallbackType === 'nested suspense') { + fallback = React.createElement( + React.Fragment, + null, + ...createRandomChildren(3), + ); + } + + return React.createElement( + Suspense, + {maxDuration, fallback}, + ...children, + ); } case 'return': default: @@ -333,6 +354,28 @@ describe('ReactSuspenseFuzz', () => { ); }); + it('hard-coded cases', () => { + const {Text, testResolvedOutput} = createFuzzer(); + + testResolvedOutput( + + + + + + + , + ); + }); + it('generative tests', () => { const {generateTestCase, testResolvedOutput} = createFuzzer(); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index de4b75337ef3f..9580c425a47b0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -937,6 +937,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); expect(ReactNoop.getChildrenAsJSX()).toEqual( +