Skip to content

Commit

Permalink
Moar fuzziness
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed Nov 8, 2018
1 parent 85d4c78 commit 01c1093
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 21 deletions.
9 changes: 5 additions & 4 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 11 additions & 7 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
Update,
NoEffect,
DidCapture,
Deletion,
} from 'shared/ReactSideEffectTags';
import invariant from 'shared/invariant';

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}

Expand Down
9 changes: 0 additions & 9 deletions packages/react-reconciler/src/ReactFiberUnwindWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import {
LOW_PRIORITY_EXPIRATION,
} from './ReactFiberExpirationTime';
import {findEarliestOutstandingPriorityLevel} from './ReactFiberPendingPriority';
import {reconcileChildren} from './ReactFiberBeginWork';

function createRootErrorUpdate(
fiber: Fiber,
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -333,6 +354,28 @@ describe('ReactSuspenseFuzz', () => {
);
});

it('hard-coded cases', () => {
const {Text, testResolvedOutput} = createFuzzer();

testResolvedOutput(
<React.Fragment>
<Text
initialDelay={20}
text="A"
updates={[{beginAfter: 10, suspendFor: 20}]}
/>
<Suspense fallback="Loading... (B)">
<Text
initialDelay={10}
text="B"
updates={[{beginAfter: 30, suspendFor: 50}]}
/>
<Text text="C" />
</Suspense>
</React.Fragment>,
);
});

it('generative tests', () => {
const {generateTestCase, testResolvedOutput} = createFuzzer();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
]);
expect(ReactNoop.getChildrenAsJSX()).toEqual(
<React.Fragment>
<span hidden={true} prop="Step: 1" />
<span hidden={true} prop="Sibling" />
<span prop="Loading (1)" />
<span prop="Loading (2)" />
Expand Down Expand Up @@ -1060,6 +1061,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
expect(ReactNoop.getChildrenAsJSX()).toEqual(
<React.Fragment>
<span hidden={true} prop="Before" />
<span hidden={true} prop="Async: 1" />
<span hidden={true} prop="After" />
<span prop="Loading..." />

Expand Down Expand Up @@ -1194,6 +1196,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
expect(ReactNoop.getChildrenAsJSX()).toEqual(
<React.Fragment>
<span hidden={true} prop="Before" />
<span hidden={true} prop="Async: 1" />
<span hidden={true} prop="After" />
<span prop="Loading..." />

Expand Down

0 comments on commit 01c1093

Please sign in to comment.