From a99a5f519d5d6553fdfb2e6d89cb37a495e44e79 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 4 Aug 2024 15:40:28 -0400 Subject: [PATCH 1/8] Mount virtual instances --- .../src/backend/fiber/renderer.js | 199 ++++++++++++++++-- .../src/frontend/types.js | 16 +- 2 files changed, 200 insertions(+), 15 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 0e80ef642a3ac..fce535f67f718 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -26,6 +26,7 @@ import { ElementTypeSuspense, ElementTypeSuspenseList, ElementTypeTracingMarker, + ElementTypeVirtual, StrictMode, } from 'react-devtools-shared/src/frontend/types'; import { @@ -134,7 +135,7 @@ import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack'; // Kinds const FIBER_INSTANCE = 0; -// const VIRTUAL_INSTANCE = 1; +const VIRTUAL_INSTANCE = 1; // Flags const FORCE_SUSPENSE_FALLBACK = /* */ 0b001; @@ -197,6 +198,24 @@ type VirtualInstance = { data: ReactComponentInfo, }; +function createVirtualInstance( + debugEntry: ReactComponentInfo, +): VirtualInstance { + return { + kind: VIRTUAL_INSTANCE, + id: getUID(), + parent: null, + firstChild: null, + previousSibling: null, + nextSibling: null, + flags: 0, + componentStack: null, + errors: null, + warnings: null, + data: debugEntry, + }; +} + type DevToolsInstance = FiberInstance | VirtualInstance; type getDisplayNameForFiberType = (fiber: Fiber) => string | null; @@ -2081,20 +2100,21 @@ export function attach( debug('recordMount()', fiber, parentInstance); } - const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); - // Adding a new field here would require a bridge protocol version bump (a backwads breaking change). - // Instead let's re-purpose a pre-existing field to carry more information. - let profilingFlags = 0; - if (isProfilingSupported) { - profilingFlags = PROFILING_FLAG_BASIC_SUPPORT; - if (typeof injectProfilingHooks === 'function') { - profilingFlags |= PROFILING_FLAG_TIMELINE_SUPPORT; + if (isRoot) { + const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); + + // Adding a new field here would require a bridge protocol version bump (a backwads breaking change). + // Instead let's re-purpose a pre-existing field to carry more information. + let profilingFlags = 0; + if (isProfilingSupported) { + profilingFlags = PROFILING_FLAG_BASIC_SUPPORT; + if (typeof injectProfilingHooks === 'function') { + profilingFlags |= PROFILING_FLAG_TIMELINE_SUPPORT; + } } - } - if (isRoot) { // Set supportsStrictMode to false for production renderer builds const isProductionBuildOfRenderer = renderer.bundleType === 0; @@ -2184,6 +2204,48 @@ export function attach( return fiberInstance; } + function recordVirtualMount( + instance: VirtualInstance, + parentInstance: DevToolsInstance | null, + ): void { + const id = instance.id; + + idToDevToolsInstanceMap.set(id, instance); + + const isProfilingSupported = false; // TODO: Support Tree Base Duration Based on Children. + + const key = null; // TODO: Track keys on ReactComponentInfo; + const displayName = instance.data.name || ''; + const elementType = ElementTypeVirtual; + // TODO: Support Virtual Owners. To do this we need to find a matching + // virtual instance which is not a super cheap parent traversal and so + // we should ideally only do that lazily. We should maybe change the + // frontend to get it lazily. + const ownerID: number = 0; + const parentID = parentInstance ? parentInstance.id : 0; + + const displayNameStringID = getStringID(displayName); + + // This check is a guard to handle a React element that has been modified + // in such a way as to bypass the default stringification of the "key" property. + const keyString = key === null ? null : String(key); + const keyStringID = getStringID(keyString); + + pushOperation(TREE_OPERATION_ADD); + pushOperation(id); + pushOperation(elementType); + pushOperation(parentID); + pushOperation(ownerID); + pushOperation(displayNameStringID); + pushOperation(keyStringID); + + if (isProfilingSupported) { + idToRootMap.set(id, currentRootID); + // TODO: Include tree base duration of children somehow. + // recordProfilingDurations(...); + } + } + function recordUnmount(fiberInstance: FiberInstance): void { const fiber = fiberInstance.data; if (__DEBUG__) { @@ -2302,17 +2364,126 @@ export function attach( } } - function mountChildrenRecursively( + function mountVirtualInstanceRecursively( + virtualInstance: VirtualInstance, + firstChild: Fiber, + lastChild: null | Fiber, // non-inclusive + traceNearestHostComponentUpdate: boolean, + virtualLevel: number, // the nth level of virtual instances + ): void { + const stashedParent = reconcilingParent; + const stashedPrevious = previouslyReconciledSibling; + const stashedRemaining = remainingReconcilingChildren; + // Push a new DevTools instance parent while reconciling this subtree. + reconcilingParent = virtualInstance; + previouslyReconciledSibling = null; + remainingReconcilingChildren = null; + try { + mountVirtualChildrenRecursively( + firstChild, + lastChild, + traceNearestHostComponentUpdate, + virtualLevel + 1, + ); + } finally { + reconcilingParent = stashedParent; + previouslyReconciledSibling = stashedPrevious; + remainingReconcilingChildren = stashedRemaining; + } + } + + function mountVirtualChildrenRecursively( firstChild: Fiber, + lastChild: null | Fiber, // non-inclusive traceNearestHostComponentUpdate: boolean, + virtualLevel: number, // the nth level of virtual instances ): void { // Iterate over siblings rather than recursing. // This reduces the chance of stack overflow for wide trees (e.g. lists with many items). let fiber: Fiber | null = firstChild; - while (fiber !== null) { - mountFiberRecursively(fiber, traceNearestHostComponentUpdate); + let previousVirtualInstance: null | VirtualInstance = null; + let previousVirtualInstanceFirstFiber: Fiber = firstChild; + while (fiber !== null && fiber !== lastChild) { + let level = 0; + if (fiber._debugInfo) { + for (let i = 0; i < fiber._debugInfo.length; i++) { + const debugEntry = fiber._debugInfo[i]; + if (typeof debugEntry.name !== 'string') { + // Not a Component. Some other Debug Info. + continue; + } + const componentInfo: ReactComponentInfo = (debugEntry: any); + if (level === virtualLevel) { + if ( + previousVirtualInstance === null || + // Consecutive children with the same debug entry as a parent gets + // treated as if they share the same virtual instance. + previousVirtualInstance.data !== debugEntry + ) { + if (previousVirtualInstance !== null) { + // Mount any previous children that should go into the previous parent. + mountVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceFirstFiber, + fiber, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } + previousVirtualInstance = createVirtualInstance(componentInfo); + recordVirtualMount(previousVirtualInstance, reconcilingParent); + insertChild(previousVirtualInstance); + previousVirtualInstanceFirstFiber = fiber; + } + level++; + break; + } else { + level++; + } + } + } + if (level === virtualLevel) { + if (previousVirtualInstance !== null) { + // If we were working on a virtual instance and this is not a virtual + // instance, then we end the sequence and mount any previous children + // that should go into the previous virtual instance. + mountVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceFirstFiber, + fiber, + traceNearestHostComponentUpdate, + virtualLevel, + ); + previousVirtualInstance = null; + } + // We've reached the end of the virtual levels, but not beyond, + // and now continue with the regular fiber. + mountFiberRecursively(fiber, traceNearestHostComponentUpdate); + } fiber = fiber.sibling; } + if (previousVirtualInstance !== null) { + // Mount any previous children that should go into the previous parent. + mountVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceFirstFiber, + null, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } + } + + function mountChildrenRecursively( + firstChild: Fiber, + traceNearestHostComponentUpdate: boolean, + ): void { + mountVirtualChildrenRecursively( + firstChild, + null, + traceNearestHostComponentUpdate, + 0, // first level + ); } function mountFiberRecursively( diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index b2e7357970d00..05fa9e9333381 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -48,11 +48,25 @@ export const ElementTypeRoot = 11; export const ElementTypeSuspense = 12; export const ElementTypeSuspenseList = 13; export const ElementTypeTracingMarker = 14; +export const ElementTypeVirtual = 15; // Different types of elements displayed in the Elements tree. // These types may be used to visually distinguish types, // or to enable/disable certain functionality. -export type ElementType = 1 | 2 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14; +export type ElementType = + | 1 + | 2 + | 5 + | 6 + | 7 + | 8 + | 9 + | 10 + | 11 + | 12 + | 13 + | 14 + | 15; // WARNING // The values below are referenced by ComponentFilters (which are saved via localStorage). From 5402eee503cf8dbc21c613b90e1b3ebb47a10f3c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 4 Aug 2024 16:45:39 -0400 Subject: [PATCH 2/8] Unmount virtual instances Since the same instance can have multiple child fibers, we need a ref count to know when it's safe to delete. --- .../src/backend/fiber/renderer.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index fce535f67f718..2ed5ef1a36dba 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2323,6 +2323,15 @@ export function attach( function removeChild(instance: DevToolsInstance): void { if (instance.parent === null) { + if (remainingReconcilingChildren === instance) { + throw new Error( + 'Remaining children should not have items with no parent', + ); + } else if (instance.nextSibling !== null) { + throw new Error('A deleted instance should not have previous siblings'); + } else if (instance.previousSibling !== null) { + throw new Error('A deleted instance should not have previous siblings'); + } // Already deleted. return; } @@ -2392,6 +2401,22 @@ export function attach( } } + function recordVirtualUnmount(instance: VirtualInstance) { + if (trackedPathMatchFiber !== null) { + // We're in the process of trying to restore previous selection. + // TODO: Handle virtual instances on the tracked path. + } + + const id = instance.id; + pendingRealUnmountedIDs.push(id); + + const isProfilingSupported = false; // TODO: Profiling support. + if (isProfilingSupported) { + idToRootMap.delete(id); + idToTreeBaseDurationMap.delete(id); + } + } + function mountVirtualChildrenRecursively( firstChild: Fiber, lastChild: null | Fiber, // non-inclusive @@ -2616,6 +2641,8 @@ export function attach( } if (instance.kind === FIBER_INSTANCE) { recordUnmount(instance); + } else { + recordVirtualUnmount(instance); } removeChild(instance); } From 9356b0304d53ce5eed7b2aed0cb16469110928e2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 4 Aug 2024 18:50:37 -0400 Subject: [PATCH 3/8] Update virtual instances --- .../src/backend/fiber/renderer.js | 257 +++++++++++++++--- 1 file changed, 217 insertions(+), 40 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 2ed5ef1a36dba..99ff57fa316bc 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2631,6 +2631,7 @@ export function attach( previouslyReconciledSibling = null; // Move all the children of this instance to the remaining set. remainingReconcilingChildren = instance.firstChild; + instance.firstChild = null; try { // Unmount the remaining set. unmountRemainingChildren(); @@ -2751,57 +2752,195 @@ export function attach( } } - // Returns whether closest unfiltered fiber parent needs to reset its child list. - function updateChildrenRecursively( - nextFirstChild: null | Fiber, + function updateVirtualInstanceRecursively( + virtualInstance: VirtualInstance, + nextFirstChild: Fiber, + nextLastChild: null | Fiber, // non-inclusive + prevFirstChild: null | Fiber, + traceNearestHostComponentUpdate: boolean, + virtualLevel: number, // the nth level of virtual instances + ): void { + const stashedParent = reconcilingParent; + const stashedPrevious = previouslyReconciledSibling; + const stashedRemaining = remainingReconcilingChildren; + // Push a new DevTools instance parent while reconciling this subtree. + reconcilingParent = virtualInstance; + previouslyReconciledSibling = null; + // Move all the children of this instance to the remaining set. + // We'll move them back one by one, and anything that remains is deleted. + remainingReconcilingChildren = virtualInstance.firstChild; + virtualInstance.firstChild = null; + try { + if ( + updateVirtualChildrenRecursively( + nextFirstChild, + nextLastChild, + prevFirstChild, + traceNearestHostComponentUpdate, + virtualLevel + 1, + ) + ) { + recordResetChildren(virtualInstance); + } + } finally { + unmountRemainingChildren(); + reconcilingParent = stashedParent; + previouslyReconciledSibling = stashedPrevious; + remainingReconcilingChildren = stashedRemaining; + } + } + + function updateVirtualChildrenRecursively( + nextFirstChild: Fiber, + nextLastChild: null | Fiber, // non-inclusive prevFirstChild: null | Fiber, traceNearestHostComponentUpdate: boolean, + virtualLevel: number, // the nth level of virtual instances ): boolean { let shouldResetChildren = false; // 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 = nextFirstChild; + let nextChild: null | Fiber = nextFirstChild; let prevChildAtSameIndex = prevFirstChild; - 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 (prevChildAtSameIndex === nextChild) { - // This set is unchanged. We're just going through it to place all the - // children again. - if ( - updateFiberRecursively( - nextChild, - nextChild, - traceNearestHostComponentUpdate, - ) - ) { - throw new Error('Updating the same fiber should not cause reorder'); + let previousVirtualInstance: null | VirtualInstance = null; + let previousVirtualInstanceWasMount: boolean = false; + let previousVirtualInstanceNextFirstFiber: Fiber = nextFirstChild; + let previousVirtualInstancePrevFirstFiber: null | Fiber = prevFirstChild; + while (nextChild !== null && nextChild !== nextLastChild) { + let level = 0; + if (nextChild._debugInfo) { + for (let i = 0; i < nextChild._debugInfo.length; i++) { + const debugEntry = nextChild._debugInfo[i]; + if (typeof debugEntry.name !== 'string') { + // Not a Component. Some other Debug Info. + continue; + } + const componentInfo: ReactComponentInfo = (debugEntry: any); + if (level === virtualLevel) { + if ( + previousVirtualInstance === null || + // Consecutive children with the same debug entry as a parent gets + // treated as if they share the same virtual instance. + previousVirtualInstance.data !== componentInfo + ) { + if (previousVirtualInstance !== null) { + // Mount any previous children that should go into the previous parent. + if (previousVirtualInstanceWasMount) { + mountVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceNextFirstFiber, + nextChild, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } else { + updateVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceNextFirstFiber, + nextChild, + previousVirtualInstancePrevFirstFiber, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } + } + const firstRemainingChild = remainingReconcilingChildren; + if ( + firstRemainingChild !== null && + firstRemainingChild.kind === VIRTUAL_INSTANCE && + firstRemainingChild.data.name === componentInfo.name + ) { + // If the previous children had a virtual instance in the same slot + // with the same name, then we claim it and reuse it for this update. + // Update it with the latest entry. + firstRemainingChild.data = componentInfo; + moveChild(firstRemainingChild); + previousVirtualInstance = firstRemainingChild; + previousVirtualInstanceWasMount = false; + } else { + // Otherwise we create a new instance. + const newVirtualInstance = createVirtualInstance(componentInfo); + recordVirtualMount(newVirtualInstance, reconcilingParent); + insertChild(newVirtualInstance); + previousVirtualInstance = newVirtualInstance; + previousVirtualInstanceWasMount = true; + shouldResetChildren = true; + } + // Existing children might be reparented into this new virtual instance. + // TODO: This will cause the front end to error which needs to be fixed. + previousVirtualInstanceNextFirstFiber = nextChild; + previousVirtualInstancePrevFirstFiber = prevChildAtSameIndex; + } + level++; + break; + } else { + level++; + } } - } else if (nextChild.alternate) { - const prevChild = nextChild.alternate; - if ( - updateFiberRecursively( - nextChild, - prevChild, - traceNearestHostComponentUpdate, - ) - ) { - // If a nested tree child order changed but it can't handle its own - // child order invalidation (e.g. because it's filtered out like host nodes), - // propagate the need to reset child order upwards to this Fiber. - shouldResetChildren = true; + } + if (level === virtualLevel) { + if (previousVirtualInstance !== null) { + // If we were working on a virtual instance and this is not a virtual + // instance, then we end the sequence and update any previous children + // that should go into the previous virtual instance. + if (previousVirtualInstanceWasMount) { + mountVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceNextFirstFiber, + nextChild, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } else { + updateVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceNextFirstFiber, + nextChild, + previousVirtualInstancePrevFirstFiber, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } + previousVirtualInstance = null; } - // 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 (prevChild !== prevChildAtSameIndex) { + // We've reached the end of the virtual levels, but not beyond, + // and now continue with the regular fiber. + if (prevChildAtSameIndex === nextChild) { + // This set is unchanged. We're just going through it to place all the + // children again. + if ( + updateFiberRecursively( + nextChild, + nextChild, + traceNearestHostComponentUpdate, + ) + ) { + throw new Error('Updating the same fiber should not cause reorder'); + } + } else if (nextChild.alternate) { + const prevChild = nextChild.alternate; + if ( + updateFiberRecursively( + nextChild, + prevChild, + traceNearestHostComponentUpdate, + ) + ) { + // If a nested tree child order changed but it can't handle its own + // child order invalidation (e.g. because it's filtered out like host nodes), + // propagate the need to reset child order upwards to this Fiber. + shouldResetChildren = true; + } + // 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 (prevChild !== prevChildAtSameIndex) { + shouldResetChildren = true; + } + } else { + mountFiberRecursively(nextChild, traceNearestHostComponentUpdate); shouldResetChildren = true; } - } else { - mountFiberRecursively(nextChild, traceNearestHostComponentUpdate); - shouldResetChildren = true; } // Try the next child. nextChild = nextChild.sibling; @@ -2811,6 +2950,26 @@ export function attach( prevChildAtSameIndex = prevChildAtSameIndex.sibling; } } + if (previousVirtualInstance !== null) { + if (previousVirtualInstanceWasMount) { + mountVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceNextFirstFiber, + null, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } else { + updateVirtualInstanceRecursively( + previousVirtualInstance, + previousVirtualInstanceNextFirstFiber, + null, + previousVirtualInstancePrevFirstFiber, + traceNearestHostComponentUpdate, + virtualLevel, + ); + } + } // If we have no more children, but used to, they don't line up. if (prevChildAtSameIndex !== null) { shouldResetChildren = true; @@ -2818,6 +2977,24 @@ export function attach( return shouldResetChildren; } + // Returns whether closest unfiltered fiber parent needs to reset its child list. + function updateChildrenRecursively( + nextFirstChild: null | Fiber, + prevFirstChild: null | Fiber, + traceNearestHostComponentUpdate: boolean, + ): boolean { + if (nextFirstChild === null) { + return prevFirstChild !== null; + } + return updateVirtualChildrenRecursively( + nextFirstChild, + null, + prevFirstChild, + traceNearestHostComponentUpdate, + 0, + ); + } + // Returns whether closest unfiltered fiber parent needs to reset its child list. function updateFiberRecursively( nextFiber: Fiber, From ec79b8e1ed1e2d5516420f1eb29e0bac6ab55254 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 12 Aug 2024 20:44:08 -0400 Subject: [PATCH 4/8] Handle reparenting When a fiber moves from one virtual parent to another, we unmount the previous one and mount a new FiberInstance. This means that it is now possible for a Fiber to be associated with more than one FiberInstance at least temporarily. It also means we can't assume that just because an instance has an alternate that we have already mounted it because it might have been deleted and recreated. This makes more sense if we reconciled from the previous parent instance than the fiberToFiberInstance map. --- .../src/backend/fiber/renderer.js | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 99ff57fa316bc..d2c3c80a00573 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -1442,10 +1442,14 @@ export function attach( } } - fiberToFiberInstanceMap.delete(fiber); + if (fiberToFiberInstanceMap.get(fiber) === fiberInstance) { + fiberToFiberInstanceMap.delete(fiber); + } const {alternate} = fiber; if (alternate !== null) { - fiberToFiberInstanceMap.delete(alternate); + if (fiberToFiberInstanceMap.get(alternate) === fiberInstance) { + fiberToFiberInstanceMap.delete(alternate); + } } } @@ -2083,15 +2087,17 @@ export function attach( throw new Error('The root should have been registered at this point'); } fiberInstance = entry; - } else if ( - fiberToFiberInstanceMap.has(fiber) || - (fiber.alternate !== null && fiberToFiberInstanceMap.has(fiber.alternate)) - ) { - throw new Error('Did not expect to see this fiber being mounted twice.'); } else { fiberInstance = createFiberInstance(fiber); } + // If this already exists behind a different FiberInstance, we intentionally + // override it here to claim the fiber as part of this new instance. + // E.g. if it was part of a reparenting. fiberToFiberInstanceMap.set(fiber, fiberInstance); + const alternate = fiber.alternate; + if (alternate !== null && fiberToFiberInstanceMap.has(alternate)) { + fiberToFiberInstanceMap.set(alternate, fiberInstance); + } idToDevToolsInstanceMap.set(fiberInstance.id, fiberInstance); const id = fiberInstance.id; @@ -2848,7 +2854,8 @@ export function attach( if ( firstRemainingChild !== null && firstRemainingChild.kind === VIRTUAL_INSTANCE && - firstRemainingChild.data.name === componentInfo.name + firstRemainingChild.data.name === componentInfo.name && + firstRemainingChild.data.env === componentInfo.env ) { // If the previous children had a virtual instance in the same slot // with the same name, then we claim it and reuse it for this update. @@ -3034,18 +3041,29 @@ export function attach( const shouldIncludeInTree = !shouldFilterFiber(nextFiber); if (shouldIncludeInTree) { const entry = fiberToFiberInstanceMap.get(prevFiber); - if (entry === undefined) { - throw new Error( - 'The previous version of the fiber should have already been registered.', - ); - } - fiberInstance = entry; - // Register the new alternate in case it's not already in. - fiberToFiberInstanceMap.set(nextFiber, fiberInstance); + if (entry !== undefined && entry.parent === reconcilingParent) { + // Common case. Match in the same parent. + fiberInstance = entry; + // Register the new alternate in case it's not already in. + fiberToFiberInstanceMap.set(nextFiber, fiberInstance); + + // Update the Fiber so we that we always keep the current Fiber on the data. + fiberInstance.data = nextFiber; + moveChild(fiberInstance); + } else { + // It's possible for a FiberInstance to be reparented when virtual parents + // get their sequence split or change structure with the same render result. + // In this case we unmount the and remount the FiberInstances. + // This might cause us to lose the selection but it's an edge case. - // Update the Fiber so we that we always keep the current Fiber on the data. - fiberInstance.data = nextFiber; - moveChild(fiberInstance); + // We let the previous instance remain in the "remaining queue" it is + // in to be deleted at the end since it'll have no match. + + mountFiberRecursively(nextFiber, traceNearestHostComponentUpdate); + + // Need to mark the parent set to remount the new instance. + return true; + } if ( mostRecentlyInspectedElement !== null && From 31ae174c1c9f6a83e9f034253754a72e70a9219e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 4 Aug 2024 21:28:16 -0400 Subject: [PATCH 5/8] Badge Server Components with their environment name --- .../react-devtools-shared/src/backend/fiber/renderer.js | 7 ++++++- packages/react-devtools-shared/src/utils.js | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index d2c3c80a00573..4fb107e0405db 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2221,7 +2221,12 @@ export function attach( const isProfilingSupported = false; // TODO: Support Tree Base Duration Based on Children. const key = null; // TODO: Track keys on ReactComponentInfo; - const displayName = instance.data.name || ''; + const env = instance.data.env; + let displayName = instance.data.name || ''; + if (typeof env === 'string') { + // We model environment as an HoC name for now. + displayName = env + '(' + displayName + ')'; + } const elementType = ElementTypeVirtual; // TODO: Support Virtual Owners. To do this we need to find a matching // virtual instance which is not a super cheap parent traversal and so diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index ffbc9e390d5a4..8e6f52f6c0f8a 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -66,6 +66,7 @@ import { ElementTypeForwardRef, ElementTypeFunction, ElementTypeMemo, + ElementTypeVirtual, } from 'react-devtools-shared/src/frontend/types'; import {localStorageGetItem, localStorageSetItem} from './storage'; import {meta} from './hydration'; @@ -484,6 +485,7 @@ export function parseElementDisplayNameFromBackend( case ElementTypeForwardRef: case ElementTypeFunction: case ElementTypeMemo: + case ElementTypeVirtual: if (displayName.indexOf('(') >= 0) { const matches = displayName.match(/[^()]+/g); if (matches != null) { From 082418e71d1d1cafba838097757fedec9a93457d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 4 Aug 2024 22:08:00 -0400 Subject: [PATCH 6/8] Implement Element inspection for virtual instances --- .../src/__tests__/inspectedElement-test.js | 31 +++++ .../src/backend/fiber/renderer.js | 131 ++++++++++++++++-- 2 files changed, 149 insertions(+), 13 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index a4cc492c83efc..c77013aba3e71 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -3150,4 +3150,35 @@ describe('InspectedElement', () => { ⚠ `); }); + + // @reactVersion > 18.2 + it('should inspect server components', async () => { + const ChildPromise = Promise.resolve(
); + ChildPromise._debugInfo = [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + }, + ]; + const Parent = () => ChildPromise; + + await utils.actAsync(() => { + modernRender(); + }); + + const inspectedElement = await inspectElementAtIndex(1); + expect(inspectedElement).toMatchInlineSnapshot(` + { + "context": null, + "events": undefined, + "hooks": null, + "id": 3, + "owners": null, + "props": null, + "rootType": "createRoot()", + "state": null, + } + `); + }); }); diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 4fb107e0405db..f3323fdfd2fa5 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -4013,12 +4013,16 @@ export function attach( console.warn(`Could not find DevToolsInstance with id "${id}"`); return null; } - if (devtoolsInstance.kind !== FIBER_INSTANCE) { - // TODO: Handle VirtualInstance. - return null; + if (devtoolsInstance.kind === VIRTUAL_INSTANCE) { + return inspectVirtualInstanceRaw(devtoolsInstance); } - const fiber = - findCurrentFiberUsingSlowPathByFiberInstance(devtoolsInstance); + return inspectFiberInstanceRaw(devtoolsInstance); + } + + function inspectFiberInstanceRaw( + fiberInstance: FiberInstance, + ): InspectedElement | null { + const fiber = findCurrentFiberUsingSlowPathByFiberInstance(fiberInstance); if (fiber == null) { return null; } @@ -4221,8 +4225,10 @@ export function attach( const DidCapture = 0b000000000000000000010000000; isErrored = (fiber.flags & DidCapture) !== 0 || - (devtoolsInstance.flags & FORCE_ERROR) !== 0; - targetErrorBoundaryID = isErrored ? id : getNearestErrorBoundaryID(fiber); + (fiberInstance.flags & FORCE_ERROR) !== 0; + targetErrorBoundaryID = isErrored + ? fiberInstance.id + : getNearestErrorBoundaryID(fiber); } else { targetErrorBoundaryID = getNearestErrorBoundaryID(fiber); } @@ -4243,7 +4249,7 @@ export function attach( } return { - id, + id: fiberInstance.id, // Does the current renderer support editable hooks and function props? canEditHooks: typeof overrideHookState === 'function', @@ -4270,7 +4276,7 @@ export function attach( (!isTimedOutSuspense || // If it's showing fallback because we previously forced it to, // allow toggling it back to remove the fallback override. - (devtoolsInstance.flags & FORCE_SUSPENSE_FALLBACK) !== 0), + (fiberInstance.flags & FORCE_SUSPENSE_FALLBACK) !== 0), // Can view component source location. canViewSource, @@ -4291,13 +4297,112 @@ export function attach( props: memoizedProps, state: showState ? memoizedState : null, errors: - devtoolsInstance.errors === null + fiberInstance.errors === null + ? [] + : Array.from(fiberInstance.errors.entries()), + warnings: + fiberInstance.warnings === null + ? [] + : Array.from(fiberInstance.warnings.entries()), + + // List of owners + owners, + + rootType, + rendererPackageName: renderer.rendererPackageName, + rendererVersion: renderer.version, + + plugins, + }; + } + + function inspectVirtualInstanceRaw( + virtualInstance: VirtualInstance, + ): InspectedElement | null { + const canViewSource = false; + + const key = null; // TODO: Track keys on ReactComponentInfo; + const props = null; // TODO: Track props on ReactComponentInfo; + + const env = virtualInstance.data.env; + let displayName = virtualInstance.data.name || ''; + if (typeof env === 'string') { + // We model environment as an HoC name for now. + displayName = env + '(' + displayName + ')'; + } + + // TODO: Support Virtual Owners. + const owners: null | Array = null; + + let rootType = null; + let targetErrorBoundaryID = null; + let parent = virtualInstance.parent; + while (parent !== null) { + if (parent.kind === FIBER_INSTANCE) { + targetErrorBoundaryID = getNearestErrorBoundaryID(parent.data); + let current = parent.data; + while (current.return !== null) { + current = current.return; + } + const fiberRoot = current.stateNode; + if (fiberRoot != null && fiberRoot._debugRootType !== null) { + rootType = fiberRoot._debugRootType; + } + break; + } + parent = parent.parent; + } + + const plugins: Plugins = { + stylex: null, + }; + + // TODO: Support getting the source location from the owner stack. + const source = null; + + return { + id: virtualInstance.id, + + canEditHooks: false, + canEditFunctionProps: false, + + canEditHooksAndDeletePaths: false, + canEditHooksAndRenamePaths: false, + canEditFunctionPropsDeletePaths: false, + canEditFunctionPropsRenamePaths: false, + + canToggleError: supportsTogglingError && targetErrorBoundaryID != null, + isErrored: false, + targetErrorBoundaryID, + + canToggleSuspense: supportsTogglingSuspense, + + // Can view component source location. + canViewSource, + source, + + // Does the component have legacy context attached to it. + hasLegacyContext: false, + + key: key != null ? key : null, + + displayName: displayName, + type: ElementTypeVirtual, + + // Inspectable properties. + // TODO Review sanitization approach for the below inspectable values. + context: null, + hooks: null, + props: props, + state: null, + errors: + virtualInstance.errors === null ? [] - : Array.from(devtoolsInstance.errors.entries()), + : Array.from(virtualInstance.errors.entries()), warnings: - devtoolsInstance.warnings === null + virtualInstance.warnings === null ? [] - : Array.from(devtoolsInstance.warnings.entries()), + : Array.from(virtualInstance.warnings.entries()), // List of owners owners, From 4d1de50f39c64f81d1e5e96e6982769ba08676ab Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 13 Aug 2024 16:59:54 -0400 Subject: [PATCH 7/8] Test tree --- .../src/__tests__/store-test.js | 276 ++++++++++++++++++ 1 file changed, 276 insertions(+) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index dea60778d5884..2d3b6ed0cc0ee 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -2212,4 +2212,280 @@ describe('Store', () => { `); }); }); + + // @reactVersion > 18.2 + it('does not show server components without any children reified children', async () => { + // A Server Component that doesn't render into anything on the client doesn't show up. + const ServerPromise = Promise.resolve(null); + ServerPromise._debugInfo = [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + }, + ]; + const App = () => ServerPromise; + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + + `); + }); + + // @reactVersion > 18.2 + it('does show a server component that renders into a filtered node', async () => { + const ServerPromise = Promise.resolve(
); + ServerPromise._debugInfo = [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + }, + ]; + const App = () => ServerPromise; + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + [Server] + `); + }); + + it('can render the same server component twice', async () => { + function ClientComponent() { + return
; + } + const ServerPromise = Promise.resolve(); + ServerPromise._debugInfo = [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + }, + ]; + const App = () => ( + <> + {ServerPromise} + + {ServerPromise} + + ); + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + + ▾ [Server] + + `); + }); + + // @reactVersion > 18.2 + it('collapses multiple parent server components into one', async () => { + function ClientComponent() { + return
; + } + const ServerPromise = Promise.resolve(); + ServerPromise._debugInfo = [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + }, + ]; + const ServerPromise2 = Promise.resolve(); + ServerPromise2._debugInfo = [ + { + name: 'ServerComponent2', + env: 'Server', + owner: null, + }, + ]; + const App = ({initial}) => ( + <> + {ServerPromise} + {ServerPromise} + {ServerPromise2} + {initial ? null : ServerPromise2} + + ); + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + + ▾ [Server] + + `); + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + + ▾ [Server] + + + `); + }); + + // @reactVersion > 18.2 + it('can reparent a child when the server components change', async () => { + function ClientComponent() { + return
; + } + const ServerPromise = Promise.resolve(); + ServerPromise._debugInfo = [ + { + name: 'ServerAB', + env: 'Server', + owner: null, + }, + ]; + const ServerPromise2 = Promise.resolve(); + ServerPromise2._debugInfo = [ + { + name: 'ServerA', + env: 'Server', + owner: null, + }, + { + name: 'ServerB', + env: 'Server', + owner: null, + }, + ]; + const App = ({initial}) => (initial ? ServerPromise : ServerPromise2); + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + `); + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + ▾ [Server] + + `); + }); + + // @reactVersion > 18.2 + it('splits a server component parent when a different child appears between', async () => { + function ClientComponent() { + return
; + } + const ServerPromise = Promise.resolve(); + ServerPromise._debugInfo = [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + }, + ]; + const App = ({initial}) => + initial ? ( + <> + {ServerPromise} + {null} + {ServerPromise} + + ) : ( + <> + {ServerPromise} + + {ServerPromise} + + ); + + await actAsync(() => render()); + // Initially the Server Component only appears once because the children + // are consecutive. + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + + `); + + // Later the same instance gets split into two when it is no longer + // consecutive so we need two virtual instances to represent two parents. + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + + ▾ [Server] + + `); + }); + + // @reactVersion > 18.2 + it('can reorder keyed components', async () => { + function ClientComponent({text}) { + return
{text}
; + } + function getServerComponent(key) { + const ServerPromise = Promise.resolve( + , + ); + ServerPromise._debugInfo = [ + { + name: 'ServerComponent', + env: 'Server', + owner: null, + // TODO: Ideally the debug info should include the "key" too to + // preserve the virtual identity of the server component when + // reordered. Atm only the children of it gets reparented. + }, + ]; + return ServerPromise; + } + const set1 = ['A', 'B', 'C'].map(getServerComponent); + const set2 = ['B', 'A', 'D'].map(getServerComponent); + + const App = ({initial}) => (initial ? set1 : set2); + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + ▾ [Server] + + ▾ [Server] + + `); + + await actAsync(() => render()); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ [Server] + + ▾ [Server] + + ▾ [Server] + + `); + }); }); From f4e101bede8c7aaff9853b0ed5758218bfdc402a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 14 Aug 2024 11:16:39 -0400 Subject: [PATCH 8/8] Update copypasta error message. Co-authored-by: Ruslan Lesiutin --- packages/react-devtools-shared/src/backend/fiber/renderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index f3323fdfd2fa5..18aaf33a065ca 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2339,7 +2339,7 @@ export function attach( 'Remaining children should not have items with no parent', ); } else if (instance.nextSibling !== null) { - throw new Error('A deleted instance should not have previous siblings'); + throw new Error('A deleted instance should not have next siblings'); } else if (instance.previousSibling !== null) { throw new Error('A deleted instance should not have previous siblings'); }