diff --git a/shells/dev/app/SuspenseTree/index.js b/shells/dev/app/SuspenseTree/index.js new file mode 100644 index 0000000000..e9a1fef4d2 --- /dev/null +++ b/shells/dev/app/SuspenseTree/index.js @@ -0,0 +1,137 @@ +// @flow + +import React, { Suspense, useState } from 'react'; + +function SuspenseTree() { + return ( + <> +

Suspense

+

Primary to Fallback Cycle

+ +

Fallback to Primary Cycle

+ + + + ); +} + +function PrimaryFallbackTest({ initialSuspend }) { + const [suspend, setSuspend] = useState(initialSuspend); + const fallbackStep = useTestSequence('fallback', Fallback1, Fallback2); + const primaryStep = useTestSequence('primary', Primary1, Primary2); + return ( + <> + +
+ + {suspend ? : primaryStep} + + + ); +} + +function useTestSequence(label, T1, T2) { + let [step, setStep] = useState(0); + let next = ( + + ); + let allSteps = [ + <>{next}, + <> + {next} mount + , + <> + {next} update + , + <> + {next} several different{' '} + children + , + <> + {next} goodbye + , + ]; + return allSteps[step]; +} + +function NestedSuspenseTest() { + return ( + <> +

Nested Suspense

+ Loading outer}> + + + + ); +} + +function Parent() { + return ( +
+ Loading inner 1}> + Hello + {' '} + Loading inner 2}> + World + +
+ This will never load}> + + +
+ + + +
+ ); +} + +function LoadLater() { + const [loadChild, setLoadChild] = useState(0); + return ( + setLoadChild(true)}>Click to load + } + > + {loadChild ? ( + setLoadChild(false)}> + Loaded! Click to suspend again. + + ) : ( + + )} + + ); +} + +function Never() { + throw new Promise(resolve => {}); +} + +function Fallback1({ prop, ...rest }) { + return ; +} + +function Fallback2({ prop, ...rest }) { + return ; +} + +function Primary1({ prop, ...rest }) { + return ; +} + +function Primary2({ prop, ...rest }) { + return ; +} + +export default SuspenseTree; diff --git a/shells/dev/app/index.js b/shells/dev/app/index.js index 4857c0c2cf..3a76db783f 100644 --- a/shells/dev/app/index.js +++ b/shells/dev/app/index.js @@ -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); } diff --git a/src/backend/renderer.js b/src/backend/renderer.js index cf0b973e68..5d3c5b9991 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -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) { + function findReorderedChildrenRecursively( + fiber: Fiber, + nextChildren: Array + ) { 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) { diff --git a/src/constants.js b/src/constants.js index ef69e21760..d4ba283319 100644 --- a/src/constants.js +++ b/src/constants.js @@ -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'; diff --git a/src/devtools/store.js b/src/devtools/store.js index fdafcb3f5c..69dbf45133 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -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); diff --git a/src/devtools/views/Profiler/CommitTreeBuilder.js b/src/devtools/views/Profiler/CommitTreeBuilder.js index 0f229c2fc0..39fa6c131c 100644 --- a/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -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;