Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DevTools] Include some Filtered Fiber Instances #30865

Merged
merged 2 commits into from
Sep 4, 2024
+127 −27
Merged

[DevTools] Include some Filtered Fiber Instances #30865

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b037f03
Filtered Fiber Instances
sebmarkbage Aug 27, 2024
e974bda
Check if a filtered Fiber is directly owned by its parent
sebmarkbage Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 127 additions & 27 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ import {formatOwnerStack} from '../shared/DevToolsOwnerStack';
// Kinds
const FIBER_INSTANCE = 0;
const VIRTUAL_INSTANCE = 1;
const FILTERED_FIBER_INSTANCE = 2;

// Flags
const FORCE_SUSPENSE_FALLBACK = /* */ 0b001;
Expand All @@ -157,9 +158,9 @@ const FORCE_ERROR_RESET = /* */ 0b100;
type FiberInstance = {
kind: 0,
id: number,
parent: null | DevToolsInstance, // filtered parent, including virtual
firstChild: null | DevToolsInstance, // filtered first child, including virtual
nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
parent: null | DevToolsInstance,
firstChild: null | DevToolsInstance,
nextSibling: null | DevToolsInstance,
flags: number, // Force Error/Suspense
source: null | string | Error | Source, // source location of this component function, or owned child stack
errors: null | Map<string, number>, // error messages and count
Expand All @@ -184,6 +185,39 @@ function createFiberInstance(fiber: Fiber): FiberInstance {
};
}

type FilteredFiberInstance = {
kind: 2,
// We exclude id from the type to get errors if we try to access it.
// However it is still in the object to preserve hidden class.
// id: number,
parent: null | DevToolsInstance,
firstChild: null | DevToolsInstance,
nextSibling: null | DevToolsInstance,
flags: number, // Force Error/Suspense
source: null | string | Error | Source, // always null here.
errors: null, // error messages and count
warnings: null, // warning messages and count
treeBaseDuration: number, // the profiled time of the last render of this subtree
data: Fiber, // one of a Fiber pair
};

// This is used to represent a filtered Fiber but still lets us find its host instance.
function createFilteredFiberInstance(fiber: Fiber): FilteredFiberInstance {
return ({
kind: FILTERED_FIBER_INSTANCE,
id: 0,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm lying about the type here because we need to ensure this is a number to preserve the hidden class at runtime, but from a type perspective, we should not attempt to read the .id field from a filtered instance since from the front end's perspective, it never exists.

parent: null,
firstChild: null,
nextSibling: null,
flags: 0,
componentStack: null,
errors: null,
warnings: null,
treeBaseDuration: 0,
data: fiber,
}: any);
}

// This type represents a stateful instance of a Server Component or a Component
// that gets optimized away - e.g. call-through without creating a Fiber.
// It's basically a virtual Fiber. This is not a semantic concept in React.
Expand All @@ -192,9 +226,9 @@ function createFiberInstance(fiber: Fiber): FiberInstance {
type VirtualInstance = {
kind: 1,
id: number,
parent: null | DevToolsInstance, // filtered parent, including virtual
firstChild: null | DevToolsInstance, // filtered first child, including virtual
nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
parent: null | DevToolsInstance,
firstChild: null | DevToolsInstance,
nextSibling: null | DevToolsInstance,
flags: number,
source: null | string | Error | Source, // source location of this server component, or owned child stack
// Errors and Warnings happen per ReactComponentInfo which can appear in
Expand Down Expand Up @@ -226,7 +260,7 @@ function createVirtualInstance(
};
}

type DevToolsInstance = FiberInstance | VirtualInstance;
type DevToolsInstance = FiberInstance | VirtualInstance | FilteredFiberInstance;

type getDisplayNameForFiberType = (fiber: Fiber) => string | null;
type getTypeSymbolType = (type: any) => symbol | number;
Expand Down Expand Up @@ -736,7 +770,8 @@ const fiberToFiberInstanceMap: Map<Fiber, FiberInstance> = new Map();
// Map of id to one (arbitrary) Fiber in a pair.
// This Map is used to e.g. get the display name for a Fiber or schedule an update,
// operations that should be the same whether the current and work-in-progress Fiber is used.
const idToDevToolsInstanceMap: Map<number, DevToolsInstance> = new Map();
const idToDevToolsInstanceMap: Map<number, FiberInstance | VirtualInstance> =
new Map();

// Map of canonical HostInstances to the nearest parent DevToolsInstance.
const publicInstanceToDevToolsInstanceMap: Map<HostInstance, DevToolsInstance> =
Expand Down Expand Up @@ -1141,13 +1176,22 @@ export function attach(
function debugTree(instance: DevToolsInstance, indent: number = 0) {
if (__DEBUG__) {
const name =
(instance.kind === FIBER_INSTANCE
(instance.kind !== VIRTUAL_INSTANCE
? getDisplayNameForFiber(instance.data)
: instance.data.name) || '';
console.log(
' '.repeat(indent) + '- ' + instance.id + ' (' + name + ')',
' '.repeat(indent) +
'- ' +
(instance.kind === FILTERED_FIBER_INSTANCE ? 0 : instance.id) +
' (' +
name +
')',
'parent',
instance.parent === null ? ' ' : instance.parent.id,
instance.parent === null
? ' '
: instance.parent.kind === FILTERED_FIBER_INSTANCE
? 0
: instance.parent.id,
'next',
instance.nextSibling === null ? ' ' : instance.nextSibling.id,
);
Expand Down Expand Up @@ -2264,7 +2308,12 @@ export function attach(
ownerInstance.source = fiber._debugStack;
}
const ownerID = ownerInstance === null ? 0 : ownerInstance.id;
const parentID = parentInstance ? parentInstance.id : 0;
const parentID = parentInstance
? parentInstance.kind === FILTERED_FIBER_INSTANCE
? // A Filtered Fiber Instance will always have a Virtual Instance as a parent.
((parentInstance.parent: any): VirtualInstance).id
: parentInstance.id
: 0;

const displayNameStringID = getStringID(displayName);

Expand Down Expand Up @@ -2348,7 +2397,12 @@ export function attach(
ownerInstance.source = componentInfo.debugStack;
}
const ownerID = ownerInstance === null ? 0 : ownerInstance.id;
const parentID = parentInstance ? parentInstance.id : 0;
const parentID = parentInstance
? parentInstance.kind === FILTERED_FIBER_INSTANCE
? // A Filtered Fiber Instance will always have a Virtual Instance as a parent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this statement true only because of the current implementation (lines 2771-2772)?

Like if I've just filtered out some client component based on its name, then its not Virtual Instance only, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's only true based on the current implementation and if that changes then this, and any other place sending instruction to the frontend, would have to turn into a loop backtracking until it hit something unfiltered.

It's basically because all virtual instances themselves come from backtracking from a Fiber. It's impossible for a virtual instance to exist without bottoming out into a Fiber (for example a Server Component that renders null doesn't actually show up in the tree today but it could be modeled by an empty Fragment Fiber). They need some Fiber to find their place in the tree if we traverse filtered things like the Suspense/Error case or finding child Host Instances.

There's no need to do that for fiber instances since they already have a Fiber that we can walk and so therefore this is already a Virtual Instance.

Technically this holds true as long it's not two Filtered Fibers in a row. Once you could have two filtered fibers nested it would need to turn into a loop.

((parentInstance.parent: any): VirtualInstance).id
: parentInstance.id
: 0;

const displayNameStringID = getStringID(displayName);

Expand Down Expand Up @@ -2713,6 +2767,25 @@ export function attach(
if (shouldIncludeInTree) {
newInstance = recordMount(fiber, reconcilingParent);
insertChild(newInstance);
} else if (
reconcilingParent !== null &&
reconcilingParent.kind === VIRTUAL_INSTANCE
) {
// If the parent is a Virtual Instance and we filtered this Fiber we include a
// hidden node.

if (
reconcilingParent.data === fiber._debugOwner &&
fiber._debugStack != null &&
reconcilingParent.source === null
) {
// The new Fiber is directly owned by the parent. Therefore somewhere on the
// debugStack will be a stack frame inside parent that we can use as its soruce.
reconcilingParent.source = fiber._debugStack;
}

newInstance = createFilteredFiberInstance(fiber);
insertChild(newInstance);
}

// If we have the tree selection from previous reload, try to match this Fiber.
Expand All @@ -2725,7 +2798,7 @@ export function attach(
const stashedParent = reconcilingParent;
const stashedPrevious = previouslyReconciledSibling;
const stashedRemaining = remainingReconcilingChildren;
if (shouldIncludeInTree) {
if (newInstance !== null) {
// Push a new DevTools instance parent while reconciling this subtree.
reconcilingParent = newInstance;
previouslyReconciledSibling = null;
Expand Down Expand Up @@ -2810,7 +2883,7 @@ export function attach(
}
}
} finally {
if (shouldIncludeInTree) {
if (newInstance !== null) {
reconcilingParent = stashedParent;
previouslyReconciledSibling = stashedPrevious;
remainingReconcilingChildren = stashedRemaining;
Expand Down Expand Up @@ -2850,8 +2923,10 @@ export function attach(
}
if (instance.kind === FIBER_INSTANCE) {
recordUnmount(instance);
} else {
} else if (instance.kind === VIRTUAL_INSTANCE) {
recordVirtualUnmount(instance);
} else {
untrackFiber(instance, instance.data);
}
removeChild(instance, null);
}
Expand Down Expand Up @@ -2956,7 +3031,9 @@ export function attach(
virtualInstance.treeBaseDuration = treeBaseDuration;
}

function recordResetChildren(parentInstance: DevToolsInstance) {
function recordResetChildren(
parentInstance: FiberInstance | VirtualInstance,
) {
if (__DEBUG__) {
if (
parentInstance.firstChild !== null &&
Expand All @@ -2976,7 +3053,17 @@ export function attach(

let child: null | DevToolsInstance = parentInstance.firstChild;
while (child !== null) {
nextChildren.push(child.id);
if (child.kind === FILTERED_FIBER_INSTANCE) {
for (
let innerChild: null | DevToolsInstance = parentInstance.firstChild;
innerChild !== null;
innerChild = innerChild.nextSibling
) {
nextChildren.push((innerChild: any).id);
}
} else {
nextChildren.push(child.id);
}
child = child.nextSibling;
}

Expand Down Expand Up @@ -3788,7 +3875,7 @@ export function attach(
devtoolsInstance: DevToolsInstance,
hostInstances: Array<HostInstance>,
) {
if (devtoolsInstance.kind === FIBER_INSTANCE) {
if (devtoolsInstance.kind !== VIRTUAL_INSTANCE) {
const fiber = devtoolsInstance.data;
appendHostInstancesByFiber(fiber, hostInstances);
return;
Expand Down Expand Up @@ -3889,6 +3976,10 @@ export function attach(
): number | null {
const instance = publicInstanceToDevToolsInstanceMap.get(publicInstance);
if (instance !== undefined) {
if (instance.kind === FILTERED_FIBER_INSTANCE) {
// A Filtered Fiber Instance will always have a Virtual Instance as a parent.
return ((instance.parent: any): VirtualInstance).id;
}
return instance.id;
}
return null;
Expand Down Expand Up @@ -3941,7 +4032,7 @@ export function attach(
}

function instanceToSerializedElement(
instance: DevToolsInstance,
instance: FiberInstance | VirtualInstance,
): SerializedElement {
if (instance.kind === FIBER_INSTANCE) {
const fiber = instance.data;
Expand Down Expand Up @@ -4036,7 +4127,7 @@ export function attach(
function findNearestOwnerInstance(
parentInstance: null | DevToolsInstance,
owner: void | null | ReactComponentInfo | Fiber,
): null | DevToolsInstance {
): null | FiberInstance | VirtualInstance {
if (owner == null) {
return null;
}
Expand All @@ -4051,6 +4142,9 @@ export function attach(
// needs a duck type check anyway.
parentInstance.data === (owner: any).alternate
) {
if (parentInstance.kind === FILTERED_FIBER_INSTANCE) {
return null;
}
return parentInstance;
}
parentInstance = parentInstance.parent;
Expand Down Expand Up @@ -4128,7 +4222,11 @@ export function attach(
if (devtoolsInstance.kind === VIRTUAL_INSTANCE) {
return inspectVirtualInstanceRaw(devtoolsInstance);
}
return inspectFiberInstanceRaw(devtoolsInstance);
if (devtoolsInstance.kind === FIBER_INSTANCE) {
return inspectFiberInstanceRaw(devtoolsInstance);
}
(devtoolsInstance: FilteredFiberInstance); // assert exhaustive
throw new Error('Unsupported instance kind');
}

function inspectFiberInstanceRaw(
Expand Down Expand Up @@ -4431,7 +4529,7 @@ export function attach(
let targetErrorBoundaryID = null;
let parent = virtualInstance.parent;
while (parent !== null) {
if (parent.kind === FIBER_INSTANCE) {
if (parent.kind !== VIRTUAL_INSTANCE) {
targetErrorBoundaryID = getNearestErrorBoundaryID(parent.data);
let current = parent.data;
while (current.return !== null) {
Expand Down Expand Up @@ -5222,7 +5320,9 @@ export function attach(
) {
// We don't need to convert milliseconds to microseconds in this case,
// because the profiling summary is JSON serialized.
target.push([instance.id, instance.treeBaseDuration]);
if (instance.kind !== FILTERED_FIBER_INSTANCE) {
target.push([instance.id, instance.treeBaseDuration]);
}
for (
let child = instance.firstChild;
child !== null;
Expand Down Expand Up @@ -5436,7 +5536,7 @@ export function attach(
// In that case, we'll do some extra checks for matching mounts.
let trackedPath: Array<PathFrame> | null = null;
let trackedPathMatchFiber: Fiber | null = null; // This is the deepest unfiltered match of a Fiber.
let trackedPathMatchInstance: DevToolsInstance | null = null; // This is the deepest matched filtered Instance.
let trackedPathMatchInstance: FiberInstance | VirtualInstance | null = null; // This is the deepest matched filtered Instance.
let trackedPathMatchDepth = -1;
let mightBeOnTrackedPath = false;

Expand All @@ -5455,7 +5555,7 @@ export function attach(
// The return value signals whether we should keep matching siblings or not.
function updateTrackedPathStateBeforeMount(
fiber: Fiber,
fiberInstance: null | FiberInstance,
fiberInstance: null | FiberInstance | FilteredFiberInstance,
): boolean {
if (trackedPath === null || !mightBeOnTrackedPath) {
// Fast path: there's nothing to track so do nothing and ignore siblings.
Expand Down Expand Up @@ -5484,7 +5584,7 @@ export function attach(
) {
// We have our next match.
trackedPathMatchFiber = fiber;
if (fiberInstance !== null) {
if (fiberInstance !== null && fiberInstance.kind === FIBER_INSTANCE) {
trackedPathMatchInstance = fiberInstance;
}
trackedPathMatchDepth++;
Expand Down
Loading