Skip to content

[DevTools] Get source location from structured callsites in prepareStackTrace #33143

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.

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
May 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 12 additions & 14 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
gt,
gte,
parseSourceFromComponentStack,
parseSourceFromOwnerStack,
serializeToString,
} from 'react-devtools-shared/src/backend/utils';
import {
Expand Down Expand Up @@ -5805,15 +5806,13 @@ export function attach(
function getSourceForFiberInstance(
fiberInstance: FiberInstance,
): Source | null {
const unresolvedSource = fiberInstance.source;
if (
unresolvedSource !== null &&
typeof unresolvedSource === 'object' &&
!isError(unresolvedSource)
) {
// $FlowFixMe: isError should have refined it.
return unresolvedSource;
// Favor the owner source if we have one.
const ownerSource = getSourceForInstance(fiberInstance);
if (ownerSource !== null) {
return ownerSource;
}

// Otherwise fallback to the throwing trick.
const dispatcherRef = getDispatcherRef(renderer);
const stackFrame =
dispatcherRef == null
Expand All @@ -5824,18 +5823,15 @@ export function attach(
dispatcherRef,
);
if (stackFrame === null) {
// If we don't find a source location by throwing, try to get one
// from an owned child if possible. This is the same branch as
// for virtual instances.
return getSourceForInstance(fiberInstance);
return null;
}
const source = parseSourceFromComponentStack(stackFrame);
fiberInstance.source = source;
return source;
}

function getSourceForInstance(instance: DevToolsInstance): Source | null {
let unresolvedSource = instance.source;
const unresolvedSource = instance.source;
if (unresolvedSource === null) {
// We don't have any source yet. We can try again later in case an owned child mounts later.
// TODO: We won't have any information here if the child is filtered.
Expand All @@ -5848,7 +5844,9 @@ export function attach(
// any intermediate utility functions. This won't point to the top of the component function
// but it's at least somewhere within it.
if (isError(unresolvedSource)) {
unresolvedSource = formatOwnerStack((unresolvedSource: any));
return (instance.source = parseSourceFromOwnerStack(
(unresolvedSource: any),
));
Comment on lines +5847 to +5849
Copy link
Contributor

Choose a reason for hiding this comment

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

If you would make isError a type guard, will it be enough to remove the any type casting?

function isError(object: mixed): object is Error {
  return toString.call(object) === '[object Error]';
}

}
if (typeof unresolvedSource === 'string') {
const idx = unresolvedSource.lastIndexOf('\n');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ export function formatOwnerStack(error: Error): string {
const prevPrepareStackTrace = Error.prepareStackTrace;
// $FlowFixMe[incompatible-type] It does accept undefined.
Error.prepareStackTrace = undefined;
let stack = error.stack;
const stack = error.stack;
Error.prepareStackTrace = prevPrepareStackTrace;
return formatOwnerStackString(stack);
}

export function formatOwnerStackString(stack: string): string {
if (stack.startsWith('Error: react-stack-top-frame\n')) {
// V8's default formatting prefixes with the error message which we
// don't want/need.
Expand Down
73 changes: 73 additions & 0 deletions packages/react-devtools-shared/src/backend/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import type {DehydratedData} from 'react-devtools-shared/src/frontend/types';
export {default as formatWithStyles} from './formatWithStyles';
export {default as formatConsoleArguments} from './formatConsoleArguments';

import {formatOwnerStackString} from '../shared/DevToolsOwnerStack';

// TODO: update this to the first React version that has a corresponding DevTools backend
const FIRST_DEVTOOLS_BACKEND_LOCKSTEP_VER = '999.9.9';
export function hasAssignedBackend(version?: string): boolean {
Expand Down Expand Up @@ -345,6 +347,77 @@ export function parseSourceFromComponentStack(
return parseSourceFromFirefoxStack(componentStack);
}

let collectedLocation: Source | null = null;

function collectStackTrace(
error: Error,
structuredStackTrace: CallSite[],
): string {
Comment on lines +352 to +355
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth leaving a link to V8 docs for this - https://v8.dev/docs/stack-trace-api#customizing-stack-traces.

let result: null | Source = null;
// Collect structured stack traces from the callsites.
// We mirror how V8 serializes stack frames and how we later parse them.
for (let i = 0; i < structuredStackTrace.length; i++) {
const callSite = structuredStackTrace[i];
if (callSite.getFunctionName() === 'react-stack-bottom-frame') {
// We pick the last frame that matches before the bottom frame since
// that will be immediately inside the component as opposed to some helper.
// If we don't find a bottom frame then we bail to string parsing.
collectedLocation = result;
// Skip everything after the bottom frame since it'll be internals.
break;
} else {
const sourceURL = callSite.getScriptNameOrSourceURL();
const line =
// $FlowFixMe[prop-missing]
typeof callSite.getEnclosingLineNumber === 'function'
? (callSite: any).getEnclosingLineNumber()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the key. Now we're jumping to the beginning of the function.

: callSite.getLineNumber();
const col =
// $FlowFixMe[prop-missing]
typeof callSite.getEnclosingColumnNumber === 'function'
? (callSite: any).getEnclosingColumnNumber()
: callSite.getLineNumber();
if (!sourceURL || !line || !col) {
// Skip eval etc. without source url. They don't have location.
continue;
}
result = {
sourceURL,
line: line,
column: col,
};
}
}
// At the same time we generate a string stack trace just in case someone
// else reads it.
const name = error.name || 'Error';
const message = error.message || '';
let stack = name + ': ' + message;
for (let i = 0; i < structuredStackTrace.length; i++) {
stack += '\n at ' + structuredStackTrace[i].toString();
}
return stack;
Comment on lines +391 to +399
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get how this can be read? You patched Error.prepareStackTrace with this function, so React DevTools is essentially the only user of this while it is installed, because you are not bubbling it up to previous function declared in Error.prepareStackTrace.

}

export function parseSourceFromOwnerStack(error: Error): Source | null {
// First attempt to collected the structured data using prepareStackTrace.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// First attempt to collected the structured data using prepareStackTrace.
// First attempt to collect the structured data using prepareStackTrace.

collectedLocation = null;
const previousPrepare = Error.prepareStackTrace;
Error.prepareStackTrace = collectStackTrace;
let stack;
try {
stack = error.stack;
} finally {
Error.prepareStackTrace = previousPrepare;
}
if (collectedLocation !== null) {
return collectedLocation;
}
// Fallback to parsing the string form.
const componentStack = formatOwnerStackString(stack);
return parseSourceFromComponentStack(componentStack);
}

// 0.123456789 => 0.123
// Expects high-resolution timestamp in milliseconds, like from performance.now()
// Mainly used for optimizing the size of serialized profiling payload
Expand Down
Loading