Skip to content

Commit

Permalink
Format owner stack in captureOwnerStack
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebmarkbage
sebmarkbage committed Jul 1, 2024
1 parent eaf9c8f commit 9c6179d
Show file tree
Hide file tree
Showing 5 changed files with 423 additions and 52 deletions.
179 changes: 140 additions & 39 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1766,9 +1766,9 @@ describe('ReactDOMFizzServer', () => {
// Intentionally trigger a key warning here.
return (
<div>
{children.map(t => (
<span>{t}</span>
))}
{children.map(function mapper(t) {
return <span>{t}</span>;
})}
</div>
);
}
Expand Down Expand Up @@ -1813,11 +1813,15 @@ describe('ReactDOMFizzServer', () => {
'<%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s',
'inCorrectTag',
'\n' +
' in inCorrectTag (at **)\n' +
' in C (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
' in A (at **)',
(gate(flags => flags.enableOwnerStacks)
? ' in inCorrectTag (at **)\n' +
' in C (at **)\n' +
' in A (at **)'
: ' in inCorrectTag (at **)\n' +
' in C (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
' in A (at **)'),
);
mockError.mockClear();
} else {
Expand All @@ -1834,21 +1838,20 @@ describe('ReactDOMFizzServer', () => {
'Each child in a list should have a unique "key" prop.%s%s' +
' See https://react.dev/link/warning-keys for more information.%s',
gate(flags => flags.enableOwnerStacks)
? // We currently don't track owners in Fizz which is responsible for this frame.
''
: '\n\nCheck the top-level render call using <div>.',
? ''
: '\n\nCheck the render method of `B`.',
'',
'\n' +
' in span (at **)\n' +
// TODO: Because this validates after the div has been mounted, it is part of
// the parent stack but since owner stacks will switch to owners this goes away again.
(gate(flags => flags.enableOwnerStacks)
? ' in div (at **)\n'
: '') +
' in B (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
' in A (at **)',
? ' in span (at **)\n' +
' in mapper (at **)\n' +
' in B (at **)\n' +
' in A (at **)'
: ' in span (at **)\n' +
' in B (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
' in A (at **)'),
);
} else {
expect(mockError).not.toHaveBeenCalled();
Expand Down Expand Up @@ -6519,24 +6522,25 @@ describe('ReactDOMFizzServer', () => {
mockError(...args.map(normalizeCodeLocInfo));
};

function App() {
return (
<html>
<body>
<script>{2}</script>
<script>
{['try { foo() } catch (e) {} ;', 'try { bar() } catch (e) {} ;']}
</script>
<script>
<MyScript />
</script>
</body>
</html>
);
}

try {
await act(async () => {
const {pipe} = renderToPipeableStream(
<html>
<body>
<script>{2}</script>
<script>
{[
'try { foo() } catch (e) {} ;',
'try { bar() } catch (e) {} ;',
]}
</script>
<script>
<MyScript />
</script>
</body>
</html>,
);
const {pipe} = renderToPipeableStream(<App />);
pipe(writable);
});

Expand All @@ -6545,17 +6549,29 @@ describe('ReactDOMFizzServer', () => {
expect(mockError.mock.calls[0]).toEqual([
'A script element was rendered with %s. If script element has children it must be a single string. Consider using dangerouslySetInnerHTML or passing a plain string as children.%s',
'a number for children',
componentStack(['script', 'body', 'html']),
componentStack(
gate(flags => flags.enableOwnerStacks)
? ['script', 'App']
: ['script', 'body', 'html', 'App'],
),
]);
expect(mockError.mock.calls[1]).toEqual([
'A script element was rendered with %s. If script element has children it must be a single string. Consider using dangerouslySetInnerHTML or passing a plain string as children.%s',
'an array for children',
componentStack(['script', 'body', 'html']),
componentStack(
gate(flags => flags.enableOwnerStacks)
? ['script', 'App']
: ['script', 'body', 'html', 'App'],
),
]);
expect(mockError.mock.calls[2]).toEqual([
'A script element was rendered with %s. If script element has children it must be a single string. Consider using dangerouslySetInnerHTML or passing a plain string as children.%s',
'something unexpected for children',
componentStack(['script', 'body', 'html']),
componentStack(
gate(flags => flags.enableOwnerStacks)
? ['script', 'App']
: ['script', 'body', 'html', 'App'],
),
]);
} else {
expect(mockError.mock.calls.length).toBe(0);
Expand Down Expand Up @@ -8148,4 +8164,89 @@ describe('ReactDOMFizzServer', () => {

expect(document.body.textContent).toBe('HelloWorld');
});

// @gate __DEV__ && enableOwnerStacks
it('can get the component owner stacks during rendering in dev', async () => {
let stack;

function Foo() {
return <Bar />;
}
function Bar() {
return (
<div>
<Baz />
</div>
);
}
function Baz() {
stack = React.captureOwnerStack();
return <span>hi</span>;
}

await act(() => {
const {pipe} = renderToPipeableStream(
<div>
<Foo />
</div>,
);
pipe(writable);
});

expect(normalizeCodeLocInfo(stack)).toBe(
'\n in Bar (at **)' + '\n in Foo (at **)',
);
});

// @gate __DEV__ && enableOwnerStacks
it('can get the component owner stacks for onError in dev', async () => {
const thrownError = new Error('hi');
let caughtError;
let parentStack;
let ownerStack;

function Foo() {
return <Bar />;
}
function Bar() {
return (
<div>
<Baz />
</div>
);
}
function Baz() {
throw thrownError;
}

await expect(async () => {
await act(() => {
const {pipe} = renderToPipeableStream(
<div>
<Foo />
</div>,
{
onError(error, errorInfo) {
caughtError = error;
parentStack = errorInfo.componentStack;
ownerStack = React.captureOwnerStack();
},
},
);
pipe(writable);
});
}).rejects.toThrow(thrownError);

expect(caughtError).toBe(thrownError);
expect(normalizeCodeLocInfo(parentStack)).toBe(
'\n in Baz (at **)' +
'\n in div (at **)' +
'\n in Bar (at **)' +
'\n in Foo (at **)' +
'\n in div (at **)',
);
expect(normalizeCodeLocInfo(ownerStack)).toBe(
'\n in Bar (at **)' + '\n in Foo (at **)',
);
});
});
38 changes: 38 additions & 0 deletions packages/react-server/src/ReactFizzCallUserSpace.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import type {LazyComponent} from 'react/src/ReactLazy';

// These indirections exists so we can exclude its stack frame in DEV (and anything below it).
// TODO: Consider marking the whole bundle instead of these boundaries.

/** @noinline */
export function callComponentInDEV<Props, Arg, R>(
Component: (p: Props, arg: Arg) => R,
props: Props,
secondArg: Arg,
): R {
return Component(props, secondArg);
}

interface ClassInstance<R> {
render(): R;
}

/** @noinline */
export function callRenderInDEV<R>(instance: ClassInstance<R>): R {
return instance.render();
}

/** @noinline */
export function callLazyInitInDEV(lazy: LazyComponent<any, any>): any {
const payload = lazy._payload;
const init = lazy._init;
return init(payload);
}
89 changes: 86 additions & 3 deletions packages/react-server/src/ReactFizzComponentStack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,31 @@ import {
describeClassComponentFrame,
} from 'shared/ReactComponentStackFrame';

import {enableOwnerStacks} from 'shared/ReactFeatureFlags';

import {formatOwnerStack} from './ReactFizzOwnerStack';

// DEV-only reverse linked list representing the current component stack
type BuiltInComponentStackNode = {
tag: 0,
parent: null | ComponentStackNode,
type: string,
owner?: null | ReactComponentInfo | ComponentStackNode, // DEV only
stack?: null | Error, // DEV only
stack?: null | string | Error, // DEV only
};
type FunctionComponentStackNode = {
tag: 1,
parent: null | ComponentStackNode,
type: Function,
owner?: null | ReactComponentInfo | ComponentStackNode, // DEV only
stack?: null | Error, // DEV only
stack?: null | string | Error, // DEV only
};
type ClassComponentStackNode = {
tag: 2,
parent: null | ComponentStackNode,
type: Function,
owner?: null | ReactComponentInfo | ComponentStackNode, // DEV only
stack?: null | Error, // DEV only
stack?: null | string | Error, // DEV only
};
export type ComponentStackNode =
| BuiltInComponentStackNode
Expand Down Expand Up @@ -68,3 +72,82 @@ export function getStackByComponentStackNode(
return '\nError generating stack: ' + x.message + '\n' + x.stack;
}
}

function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string {
// We use this because we don't actually want to describe the line of the component
// but just the component name.
const name = fn ? fn.displayName || fn.name : '';
return name ? describeBuiltInComponentFrame(name) : '';
}

export function getOwnerStackByComponentStackNodeInDev(
componentStack: ComponentStackNode,
): string {
if (!enableOwnerStacks || !__DEV__) {
return '';
}
try {
let info = '';

// The owner stack of the current component will be where it was created, i.e. inside its owner.
// There's no actual name of the currently executing component. Instead, that is available
// on the regular stack that's currently executing. However, for built-ins there is no such
// named stack frame and it would be ignored as being internal anyway. Therefore we add
// add one extra frame just to describe the "current" built-in component by name.
// Similarly, if there is no owner at all, then there's no stack frame so we add the name
// of the root component to the stack to know which component is currently executing.
switch (componentStack.tag) {
case 0:
info += describeBuiltInComponentFrame(componentStack.type);
break;
case 1:
case 2:
if (!componentStack.owner) {
// Only if we have no other data about the callsite do we add
// the component name as the single stack frame.
info += describeFunctionComponentFrameWithoutLineNumber(
componentStack.type,
);
}
break;
}

let owner: void | null | ComponentStackNode | ReactComponentInfo =
componentStack;

while (owner) {
if (typeof owner.tag === 'number') {
const node: ComponentStackNode = (owner: any);
owner = node.owner;
let debugStack = node.stack;
// If we don't actually print the stack if there is no owner of this JSX element.
// In a real app it's typically not useful since the root app is always controlled
// by the framework. These also tend to have noisy stacks because they're not rooted
// in a React render but in some imperative bootstrapping code. It could be useful
// if the element was created in module scope. E.g. hoisted. We could add a a single
// stack frame for context for example but it doesn't say much if that's a wrapper.
if (owner && debugStack) {
if (typeof debugStack !== 'string') {
// Stash the formatted stack so that we can avoid redoing the filtering.
node.stack = debugStack = formatOwnerStack(debugStack);
}
if (debugStack !== '') {
info += '\n' + debugStack;
}
}
} else if (typeof owner.stack === 'string') {
// Server Component
if (owner.stack !== '') {
info += '\n' + owner.stack;
}
const componentInfo: ReactComponentInfo = (owner: any);
owner = componentInfo.owner;
} else {
break;
}
}
return info;
} catch (x) {
return '\nError generating stack: ' + x.message + '\n' + x.stack;
}
}
Loading

0 comments on commit 9c6179d

Please sign in to comment.