Merge pull request #71 from gaearon/suspense-special

Fix Suspense fragment edge cases
This commit is contained in:
Dan Abramov
2019-04-05 23:59:17 +01:00
committed by GitHub
6 changed files with 450 additions and 140 deletions
+137
View File
@@ -0,0 +1,137 @@
// @flow
import React, { Suspense, useState } from 'react';
function SuspenseTree() {
return (
<>
<h1>Suspense</h1>
<h4>Primary to Fallback Cycle</h4>
<PrimaryFallbackTest initialSuspend={false} />
<h4>Fallback to Primary Cycle</h4>
<PrimaryFallbackTest initialSuspend={true} />
<NestedSuspenseTest />
</>
);
}
function PrimaryFallbackTest({ initialSuspend }) {
const [suspend, setSuspend] = useState(initialSuspend);
const fallbackStep = useTestSequence('fallback', Fallback1, Fallback2);
const primaryStep = useTestSequence('primary', Primary1, Primary2);
return (
<>
<label>
<input
checked={suspend}
onChange={e => setSuspend(e.target.checked)}
type="checkbox"
/>
Suspend
</label>
<br />
<Suspense fallback={fallbackStep}>
{suspend ? <Never /> : primaryStep}
</Suspense>
</>
);
}
function useTestSequence(label, T1, T2) {
let [step, setStep] = useState(0);
let next = (
<button onClick={() => setStep(s => (s + 1) % allSteps.length)}>
next {label} content
</button>
);
let allSteps = [
<>{next}</>,
<>
{next} <T1 prop={step}>mount</T1>
</>,
<>
{next} <T1 prop={step}>update</T1>
</>,
<>
{next} <T2 prop={step}>several</T2> <T1 prop={step}>different</T1>{' '}
<T2 prop={step}>children</T2>
</>,
<>
{next} <T2 prop={step}>goodbye</T2>
</>,
];
return allSteps[step];
}
function NestedSuspenseTest() {
return (
<>
<h3>Nested Suspense</h3>
<Suspense fallback={<Fallback1>Loading outer</Fallback1>}>
<Parent />
</Suspense>
</>
);
}
function Parent() {
return (
<div>
<Suspense fallback={<Fallback1>Loading inner 1</Fallback1>}>
<Primary1>Hello</Primary1>
</Suspense>{' '}
<Suspense fallback={<Fallback2>Loading inner 2</Fallback2>}>
<Primary2>World</Primary2>
</Suspense>
<br />
<Suspense fallback={<Fallback1>This will never load</Fallback1>}>
<Never />
</Suspense>
<br />
<b>
<LoadLater />
</b>
</div>
);
}
function LoadLater() {
const [loadChild, setLoadChild] = useState(0);
return (
<Suspense
fallback={
<Fallback1 onClick={() => setLoadChild(true)}>Click to load</Fallback1>
}
>
{loadChild ? (
<Primary1 onClick={() => setLoadChild(false)}>
Loaded! Click to suspend again.
</Primary1>
) : (
<Never />
)}
</Suspense>
);
}
function Never() {
throw new Promise(resolve => {});
}
function Fallback1({ prop, ...rest }) {
return <span {...rest} />;
}
function Fallback2({ prop, ...rest }) {
return <span {...rest} />;
}
function Primary1({ prop, ...rest }) {
return <span {...rest} />;
}
function Primary2({ prop, ...rest }) {
return <span {...rest} />;
}
export default SuspenseTree;
+2
View File
@@ -11,6 +11,7 @@ import InspectableElements from './InspectableElements';
import InteractionTracing from './InteractionTracing';
import ToDoList from './ToDoList';
import Toggle from './Toggle';
import SuspenseTree from './SuspenseTree';
import './styles.css';
@@ -33,6 +34,7 @@ function mountTestApp() {
mountHelper(ElementTypes);
mountHelper(EditableProps);
mountHelper(Toggle);
mountHelper(SuspenseTree);
mountHelper(DeeplyNestedComponents);
}
+183 -96
View File
@@ -22,6 +22,7 @@ import {
TREE_OPERATION_ADD,
TREE_OPERATION_REMOVE,
TREE_OPERATION_RESET_CHILDREN,
TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN,
TREE_OPERATION_UPDATE_TREE_BASE_DURATION,
} from '../constants';
import { getUID } from '../utils';
@@ -627,7 +628,7 @@ export function attach(
pendingOperations = new Uint32Array(0);
}
function enqueueMount(fiber: Fiber, parentFiber: Fiber | null) {
function recordMount(fiber: Fiber, parentFiber: Fiber | null) {
const isRoot = fiber.tag === HostRoot;
const id = getFiberID(getPrimaryFiber(fiber));
@@ -713,9 +714,19 @@ export function attach(
}
}
function enqueueUnmount(fiber) {
function recordUnmount(fiber: Fiber) {
const isRoot = fiber.tag === HostRoot;
const primaryFiber = getPrimaryFiber(fiber);
if (!fiberToIDMap.has(primaryFiber)) {
// If we've never seen this Fiber, it might be because
// it is inside a non-current Suspense fragment tree,
// and so the store is not even aware of it.
// In that case we can just ignore it, or otherwise
// there will be errors later on.
primaryFibers.delete(primaryFiber);
// TODO: this is fragile and can obscure actual bugs.
return;
}
const id = getFiberID(primaryFiber);
if (isRoot) {
const operation = new Uint32Array(2);
@@ -743,35 +754,80 @@ export function attach(
}
}
function mountFiber(
function recordRecursiveRemoveChildren(fiber) {
const primaryFiber = getPrimaryFiber(fiber);
const id = getFiberID(primaryFiber);
const operation = new Uint32Array(2);
operation[0] = TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN;
operation[1] = id;
addOperation(operation, false);
}
function mountFiberRecursively(
fiber: Fiber,
parentFiber: Fiber | null,
traverseSiblings = false
) {
if (__DEBUG__) {
debug('mountFiber()', fiber, parentFiber);
debug('mountFiberRecursively()', fiber, parentFiber);
}
const shouldEnqueueMount = !shouldFilterFiber(fiber);
if (shouldEnqueueMount) {
enqueueMount(fiber, parentFiber);
const shouldIncludeInTree = !shouldFilterFiber(fiber);
if (shouldIncludeInTree) {
recordMount(fiber, parentFiber);
}
if (fiber.child !== null) {
mountFiber(fiber.child, shouldEnqueueMount ? fiber : parentFiber, true);
const isTimedOutSuspense =
fiber.tag === ReactTypeOfWork.SuspenseComponent &&
fiber.memoizedState !== null;
if (isTimedOutSuspense) {
// Special case: if Suspense mounts in a timed-out state,
// get the fallback child from the inner fragment and mount
// it as if it was our own child. Updates handle this too.
const primaryChildFragment = fiber.child;
const fallbackChildFragment = primaryChildFragment.sibling;
const fallbackChild = fallbackChildFragment.child;
if (fallbackChild !== null) {
mountFiberRecursively(
fallbackChild,
shouldIncludeInTree ? fiber : parentFiber,
true
);
}
} else {
if (fiber.child !== null) {
mountFiberRecursively(
fiber.child,
shouldIncludeInTree ? fiber : parentFiber,
true
);
}
}
if (traverseSiblings && fiber.sibling !== null) {
mountFiber(fiber.sibling, parentFiber, true);
mountFiberRecursively(fiber.sibling, parentFiber, true);
}
}
function enqueueUpdateIfNecessary(
fiber: Fiber,
hasChildOrderChanged: boolean
) {
function unmountFiberRecursively(fiber, traverseSiblings = false) {
if (__DEBUG__) {
debug('enqueueUpdateIfNecessary()', fiber);
debug('unmountFiberRecursively()', fiber, traverseSiblings);
}
if (!shouldFilterFiber(fiber)) {
recordUnmount(fiber);
}
if (fiber.child !== null) {
unmountFiberRecursively(fiber.child, true);
}
if (traverseSiblings && fiber.sibling !== null) {
unmountFiberRecursively(fiber.sibling, true);
}
}
function maybeRecordUpdate(fiber: Fiber, hasChildOrderChanged: boolean) {
if (__DEBUG__) {
debug('maybeRecordUpdate()', fiber);
}
const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration');
@@ -818,7 +874,7 @@ export function attach(
// We might want to revisit this if it proves to be too inefficient.
let child = fiber.child;
while (child !== null) {
findReorderedChildren(child, nextChildren);
findReorderedChildrenRecursively(child, nextChildren);
child = child.sibling;
}
@@ -832,109 +888,140 @@ export function attach(
}
}
function findReorderedChildren(fiber: Fiber, nextChildren: Array<number>) {
function findReorderedChildrenRecursively(
fiber: Fiber,
nextChildren: Array<number>
) {
if (!shouldFilterFiber(fiber)) {
nextChildren.push(getFiberID(getPrimaryFiber(fiber)));
} else {
let child = fiber.child;
while (child !== null) {
findReorderedChildren(child, nextChildren);
findReorderedChildrenRecursively(child, nextChildren);
child = child.sibling;
}
}
}
function updateFiber(
function updateFiberRecursively(
nextFiber: Fiber,
prevFiber: Fiber,
parentFiber: Fiber | null
) {
if (__DEBUG__) {
debug('enqueueUpdateIfNecessary()', nextFiber, parentFiber);
debug('updateFiberRecursively()', nextFiber, parentFiber);
}
const shouldEnqueueUpdate = !shouldFilterFiber(nextFiber);
// The behavior of timed-out Suspense trees is unique.
// Rather than unmount the timed out content (and possibly lose important state),
// React re-parents this content within a hidden Fragment while the fallback is showing.
// This behavior doesn't need to be observable in the DevTools though.
// It might even result in a bad user experience for e.g. node selection in the Elements panel.
// The easiest fix is to strip out the intermediate Fragment fibers,
// so the Elements panel and Profiler don't need to special case them.
if (nextFiber.tag === SuspenseComponent) {
// Suspense components only have a non-null memoizedState if they're timed-out.
const prevDidTimeout = prevFiber.memoizedState !== null;
const nextDidTimeOut = nextFiber.memoizedState !== null;
// Suspense components only have a non-null memoizedState if they're timed-out.
const isTimedOutSuspense =
nextFiber.tag === SuspenseComponent && nextFiber.memoizedState !== null;
if (isTimedOutSuspense) {
// The behavior of timed-out Suspense trees is unique.
// Rather than unmount the timed out content (and possibly lose important state),
// React re-parents this content within a hidden Fragment while the fallback is showing.
// This behavior doesn't need to be observable in the DevTools though.
// It might even result in a bad user experience for e.g. node selection in the Elements panel.
// The easiest fix is to strip out the intermediate Fragment fibers,
// so the Elements panel and Profiler don't need to special case them.
const primaryChildFragment = nextFiber.child;
const fallbackChildFragment = primaryChildFragment.sibling;
const fallbackChild = fallbackChildFragment.child;
// The primary, hidden child is never actually updated in this case,
// so we can skip any updates to its tree.
// We only need to track updates to the Fallback UI for now.
if (fallbackChild.alternate) {
updateFiber(fallbackChild, fallbackChild.alternate, nextFiber);
// The logic below is inspired by the codepaths in updateSuspenseComponent()
// inside ReactFiberBeginWork in the React source code.
if (prevDidTimeout) {
if (nextDidTimeOut) {
// Fallback -> Fallback:
// 1. Reconcile fallback set.
const nextFallbackChildSet = nextFiber.child.sibling;
// Note: We can't use nextFiber.child.sibling.alternate
// because the set is special and alternate may not exist.
const prevFallbackChildSet = prevFiber.child.sibling;
updateFiberRecursively(
nextFallbackChildSet,
prevFallbackChildSet,
nextFiber
);
return;
} else {
// Fallback -> Primary:
// 1. Unmount fallback set
// Note: don't emulate fallback unmount because React actually did it.
// 2. Mount primary set
const nextPrimaryChildSet = nextFiber.child;
mountFiberRecursively(nextPrimaryChildSet, nextFiber, true);
return;
}
} else {
mountFiber(fallbackChild, nextFiber);
if (nextDidTimeOut) {
// Primary -> Fallback:
// 1. Hide primary set
// This is not a real unmount, so it won't get reported by React.
// By this point it's *too late* to find the previous primary child set
// so we'll just tell the store to "forget" about those children.
// They might "resurface" later when we switch to primary content,
// but from the store's point of view they will be a new tree.
recordRecursiveRemoveChildren(nextFiber);
// 2. Mount fallback set
const nextFallbackChildSet = nextFiber.child.sibling;
mountFiberRecursively(nextFallbackChildSet, nextFiber, true);
return;
} else {
// Primary -> Primary:
// 1. Reconcile primary set.
// Note: no return so we can passthrough to the logic below.
}
}
}
if (shouldEnqueueUpdate) {
enqueueUpdateIfNecessary(nextFiber, false);
}
} else {
let hasChildOrderChanged = false;
if (nextFiber.child !== prevFiber.child) {
// If the first child is different, we need to traverse them.
// Each next child will be either a new child (mount) or an alternate (update).
let nextChild = nextFiber.child;
let prevChildAtSameIndex = prevFiber.child;
while (nextChild) {
// We already know children will be referentially different because
// they are either new mounts or alternates of previous children.
// Schedule updates and mounts depending on whether alternates exist.
// We don't track deletions here because they are reported separately.
if (nextChild.alternate) {
const prevChild = nextChild.alternate;
updateFiber(
nextChild,
prevChild,
shouldEnqueueUpdate ? nextFiber : parentFiber
);
// However we also keep track if the order of the children matches
// the previous order. They are always different referentially, but
// if the instances line up conceptually we'll want to know that.
if (!hasChildOrderChanged && prevChild !== prevChildAtSameIndex) {
hasChildOrderChanged = true;
}
} else {
mountFiber(
nextChild,
shouldEnqueueUpdate ? nextFiber : parentFiber
);
if (!hasChildOrderChanged) {
hasChildOrderChanged = true;
}
const shouldIncludeInTree = !shouldFilterFiber(nextFiber);
let hasChildOrderChanged = false;
if (nextFiber.child !== prevFiber.child) {
// If the first child is different, we need to traverse them.
// Each next child will be either a new child (mount) or an alternate (update).
let nextChild = nextFiber.child;
let prevChildAtSameIndex = prevFiber.child;
while (nextChild) {
// We already know children will be referentially different because
// they are either new mounts or alternates of previous children.
// Schedule updates and mounts depending on whether alternates exist.
// We don't track deletions here because they are reported separately.
if (nextChild.alternate) {
const prevChild = nextChild.alternate;
updateFiberRecursively(
nextChild,
prevChild,
shouldIncludeInTree ? nextFiber : parentFiber
);
// However we also keep track if the order of the children matches
// the previous order. They are always different referentially, but
// if the instances line up conceptually we'll want to know that.
if (!hasChildOrderChanged && prevChild !== prevChildAtSameIndex) {
hasChildOrderChanged = true;
}
// Try the next child.
nextChild = nextChild.sibling;
// Advance the pointer in the previous list so that we can
// keep comparing if they line up.
if (!hasChildOrderChanged && prevChildAtSameIndex != null) {
prevChildAtSameIndex = prevChildAtSameIndex.sibling;
} else {
mountFiberRecursively(
nextChild,
shouldIncludeInTree ? nextFiber : parentFiber
);
if (!hasChildOrderChanged) {
hasChildOrderChanged = true;
}
}
// If we have no more children, but used to, they don't line up.
// Try the next child.
nextChild = nextChild.sibling;
// Advance the pointer in the previous list so that we can
// keep comparing if they line up.
if (!hasChildOrderChanged && prevChildAtSameIndex != null) {
hasChildOrderChanged = true;
prevChildAtSameIndex = prevChildAtSameIndex.sibling;
}
}
if (shouldEnqueueUpdate) {
enqueueUpdateIfNecessary(nextFiber, hasChildOrderChanged);
// If we have no more children, but used to, they don't line up.
if (!hasChildOrderChanged && prevChildAtSameIndex != null) {
hasChildOrderChanged = true;
}
}
if (shouldIncludeInTree) {
maybeRecordUpdate(nextFiber, hasChildOrderChanged);
}
}
function cleanup() {
@@ -976,7 +1063,7 @@ export function attach(
};
}
mountFiber(root.current, null);
mountFiberRecursively(root.current, null);
flushPendingEvents(root);
currentRootID = -1;
});
@@ -987,7 +1074,7 @@ export function attach(
// This is not recursive.
// We can't traverse fibers after unmounting so instead
// we rely on React telling us about each unmount.
enqueueUnmount(fiber);
recordUnmount(fiber);
}
function handleCommitFiberRoot(root) {
@@ -1021,17 +1108,17 @@ export function attach(
current.memoizedState != null && current.memoizedState.element != null;
if (!wasMounted && isMounted) {
// Mount a new root.
mountFiber(current, null);
mountFiberRecursively(current, null);
} else if (wasMounted && isMounted) {
// Update an existing root.
updateFiber(current, alternate, null);
updateFiberRecursively(current, alternate, null);
} else if (wasMounted && !isMounted) {
// Unmount an existing root.
enqueueUnmount(current);
recordUnmount(current);
}
} else {
// Mount a new root.
mountFiber(current, null);
mountFiberRecursively(current, null);
}
if (isProfiling) {
+1
View File
@@ -4,6 +4,7 @@ export const TREE_OPERATION_ADD = 1;
export const TREE_OPERATION_REMOVE = 2;
export const TREE_OPERATION_RESET_CHILDREN = 3;
export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4;
export const TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN = 5;
export const LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY =
'React::DevTools::reloadAndProfile';
+51 -4
View File
@@ -3,6 +3,7 @@
import EventEmitter from 'events';
import {
TREE_OPERATION_ADD,
TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN,
TREE_OPERATION_REMOVE,
TREE_OPERATION_RESET_CHILDREN,
TREE_OPERATION_UPDATE_TREE_BASE_DURATION,
@@ -572,7 +573,52 @@ export default class Store extends EventEmitter {
weightDelta = 1;
}
break;
case TREE_OPERATION_REMOVE:
case TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN: {
id = ((operations[i + 1]: any): number);
if (!this._idToElement.has(id)) {
throw new Error(
'Store does not contain fiber ' +
id +
'. This is a bug in React DevTools.'
);
}
i = i + 2;
let justRemovedIDs = [];
const recursivelyRemove = childID => {
justRemovedIDs.push(childID);
const child = this._idToElement.get(childID);
if (!child) {
throw new Error(
'Store does not contain fiber ' +
childID +
'. This is a bug in React DevTools.'
);
}
this._idToElement.delete(childID);
child.children.forEach(recursivelyRemove);
};
// Track removed items so search results can be updated
const oldRemovedElementIDs = removedElementIDs;
removedElementIDs = new Uint32Array(
removedElementIDs.length + justRemovedIDs.length
);
removedElementIDs.set(oldRemovedElementIDs);
let startIndex = oldRemovedElementIDs.length;
for (let j = 0; j < justRemovedIDs.length; j++) {
removedElementIDs[startIndex + j] = oldRemovedElementIDs[j];
}
parentElement = ((this._idToElement.get(id): any): Element);
parentElement.children.forEach(recursivelyRemove);
parentElement.children = [];
weightDelta = -parentElement.weight + 1;
break;
}
case TREE_OPERATION_REMOVE: {
id = ((operations[i + 1]: any): number);
if (!this._idToElement.has(id)) {
@@ -614,11 +660,12 @@ export default class Store extends EventEmitter {
}
// Track removed items so search results can be updated
const oldRemovededElementIDs = removedElementIDs;
const oldRemovedElementIDs = removedElementIDs;
removedElementIDs = new Uint32Array(removedElementIDs.length + 1);
removedElementIDs.set(oldRemovededElementIDs);
removedElementIDs[oldRemovededElementIDs.length] = id;
removedElementIDs.set(oldRemovedElementIDs);
removedElementIDs[oldRemovedElementIDs.length] = id;
break;
}
case TREE_OPERATION_RESET_CHILDREN:
id = ((operations[i + 1]: any): number);
const numChildren = ((operations[i + 2]: any): number);
@@ -3,6 +3,7 @@
import {
__DEBUG__,
TREE_OPERATION_ADD,
TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN,
TREE_OPERATION_REMOVE,
TREE_OPERATION_RESET_CHILDREN,
TREE_OPERATION_UPDATE_TREE_BASE_DURATION,
@@ -183,6 +184,14 @@ function updateTree(
i = i + 3;
if (nodes.has(id)) {
throw new Error(
'Commit tree already contains fiber ' +
id +
'. This is a bug in React DevTools.'
);
}
if (type === ElementTypeRoot) {
i++; // supportsProfiling flag
@@ -190,22 +199,16 @@ function updateTree(
debug('Add', `new root fiber ${id}`);
}
if (nodes.has(id)) {
// The renderer's tree walking approach sometimes mounts the same Fiber twice with Suspense and Lazy.
// For now, we avoid adding it to the tree twice by checking if it's already been mounted.
// Maybe in the future we'll revisit this.
} else {
const node: Node = {
children: [],
displayName: null,
id,
key: null,
parentID: 0,
treeBaseDuration: 0, // This will be updated by a subsequent operation
};
const node: Node = {
children: [],
displayName: null,
id,
key: null,
parentID: 0,
treeBaseDuration: 0, // This will be updated by a subsequent operation
};
nodes.set(id, node);
}
nodes.set(id, node);
} else {
parentID = ((operations[i]: any): number);
i++;
@@ -230,39 +233,72 @@ function updateTree(
: utfDecodeString((operations.slice(i, i + keyLength): any));
i += +keyLength;
if (nodes.has(id)) {
// The renderer's tree walking approach sometimes mounts the same Fiber twice with Suspense and Lazy.
// For now, we avoid adding it to the tree twice by checking if it's already been mounted.
// Maybe in the future we'll revisit this.
} else {
if (__DEBUG__) {
debug(
'Add',
`fiber ${id} (${displayName || 'null'}) as child of ${parentID}`
);
}
parentNode = getClonedNode(parentID);
parentNode.children = parentNode.children.concat(id);
const node: Node = {
children: [],
displayName,
id,
key,
parentID,
treeBaseDuration: 0, // This will be updated by a subsequent operation
};
nodes.set(id, node);
if (__DEBUG__) {
debug(
'Add',
`fiber ${id} (${displayName || 'null'}) as child of ${parentID}`
);
}
parentNode = getClonedNode(parentID);
parentNode.children = parentNode.children.concat(id);
const node: Node = {
children: [],
displayName,
id,
key,
parentID,
treeBaseDuration: 0, // This will be updated by a subsequent operation
};
nodes.set(id, node);
}
break;
case TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN:
id = ((operations[i + 1]: any): number);
i = i + 2;
if (!nodes.has(id)) {
throw new Error(
'Commit tree does not contain fiber ' +
id +
'. This is a bug in React DevTools.'
);
}
node = getClonedNode(id);
const recursivelyRemove = childID => {
if (!nodes.has(id)) {
throw new Error(
'Commit tree does not contain fiber ' +
id +
'. This is a bug in React DevTools.'
);
}
const child = getClonedNode(childID);
nodes.delete(childID);
child.children.forEach(recursivelyRemove);
};
node.children.forEach(recursivelyRemove);
node.children = [];
break;
case TREE_OPERATION_REMOVE:
id = ((operations[i + 1]: any): number);
i = i + 2;
if (!nodes.has(id)) {
throw new Error(
'Commit tree does not contain fiber ' +
id +
'. This is a bug in React DevTools.'
);
}
node = getClonedNode(id);
parentID = node.parentID;