Skip to content

[Fizz] Implement debugInfo #30174

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

Merged
merged 2 commits into from
Jul 2, 2024
Merged

[Fizz] Implement debugInfo #30174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32f451b
Implement debugInfo in Fizz
sebmarkbage Jul 1, 2024
11fb115
Gate Server Component stack frame outside of DEV
sebmarkbage Jul 2, 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
18 changes: 10 additions & 8 deletions packages/react-html/src/__tests__/ReactHTMLServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,17 +244,19 @@ if (!__EXPERIMENTAL__) {
expect(caughtErrors.length).toBe(1);
expect(caughtErrors[0].error).toBe(thrownError);
expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe(
// TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
// it doesn't have the Server Components in the parent stacks.
'\n in Lazy (at **)' +
'\n in div (at **)' +
'\n in div (at **)',
__DEV__
? '\n in Baz (at **)' +
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These don't have line numbers because they're server components and we don't give those line numbers for parent stacks.

'\n in div (at **)' +
'\n in Bar (at **)' +
'\n in Foo (at **)' +
'\n in div (at **)'
: '\n in Lazy (at **)' +
'\n in div (at **)' +
'\n in div (at **)',
);
expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
__DEV__ && gate(flags => flags.enableOwnerStacks)
? // TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
// it doesn't have the Server Components in the parent stacks.
'\n in Lazy (at **)'
? '\n in Bar (at **)' + '\n in Foo (at **)'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that the actual paths here point to fake evals which needs to be source mapped to make sense since this is the "client" stacks showing.

An alternative would be to snapshot the stacks on the "server" and then show it here. However, the server likely is compiled and needs to be source mapped anyway so might not matter much.

: null,
);
});
Expand Down
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/ReactFiberComponentStack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,11 @@ export function getOwnerStackByFiberInDev(
// another code path anyway. I.e. this is likely NOT a V8 based browser.
// This will cause some of the stack to have different formatting.
// TODO: Normalize server component stacks to the client formatting.
if (owner.stack !== '') {
info += '\n' + owner.stack;
const ownerStack: string = owner.stack;
owner = owner.owner;
if (owner && ownerStack !== '') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For client components, we only included the stack if there was an owner (to eliminate some of the initial bootstrapping stacks) but we didn't do the same for server components. This fixes that in Fiber and Fizz.

info += '\n' + ownerStack;
}
const componentInfo: ReactComponentInfo = (owner: any);
owner = componentInfo.owner;
} else {
break;
}
Expand Down
29 changes: 24 additions & 5 deletions packages/react-server/src/ReactFizzComponentStack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,19 @@ type ClassComponentStackNode = {
owner?: null | ReactComponentInfo | ComponentStackNode, // DEV only
stack?: null | string | Error, // DEV only
};
type ServerComponentStackNode = {
// DEV only
tag: 3,
parent: null | ComponentStackNode,
type: string, // name + env
owner?: null | ReactComponentInfo | ComponentStackNode, // DEV only
stack?: null | string | Error, // DEV only
};
export type ComponentStackNode =
| BuiltInComponentStackNode
| FunctionComponentStackNode
| ClassComponentStackNode;
| ClassComponentStackNode
| ServerComponentStackNode;

export function getStackByComponentStackNode(
componentStack: ComponentStackNode,
Expand All @@ -63,6 +72,11 @@ export function getStackByComponentStackNode(
case 2:
info += describeClassComponentFrame(node.type);
break;
case 3:
if (__DEV__) {
info += describeBuiltInComponentFrame(node.type);
break;
}
}
// $FlowFixMe[incompatible-type] we bail out when we get a null
node = node.parent;
Expand Down Expand Up @@ -110,6 +124,11 @@ export function getOwnerStackByComponentStackNodeInDev(
);
}
break;
case 3:
if (!componentStack.owner) {
info += describeBuiltInComponentFrame(componentStack.type);
}
break;
}

let owner: void | null | ComponentStackNode | ReactComponentInfo =
Expand Down Expand Up @@ -137,11 +156,11 @@ export function getOwnerStackByComponentStackNodeInDev(
}
} else if (typeof owner.stack === 'string') {
// Server Component
if (owner.stack !== '') {
info += '\n' + owner.stack;
const ownerStack: string = owner.stack;
owner = owner.owner;
if (owner && ownerStack !== '') {
info += '\n' + ownerStack;
}
const componentInfo: ReactComponentInfo = (owner: any);
owner = componentInfo.owner;
} else {
break;
}
Expand Down
Loading
Loading