Skip to content

Commit 997c7bc

Browse files
authored
[DevTools] Get source location from structured callsites in prepareStackTrace (facebook#33143)
When we get the source location for "View source for this element" we should be using the enclosing function of the callsite of the child. So that we don't just point to some random line within the component. This is similar to the technique in facebook#33136. This technique is now really better than the fake throw technique, when available. So I now favor the owner technique. The only problem it's only available in DEV and only if it has a child that's owned (and not filtered). We could implement this same technique for the error that's thrown in the fake throwing solution. However, we really shouldn't need that at all because for client components we should be able to call `inspect(fn)` at least in Chrome which is even better.
1 parent b94603b commit 997c7bc

File tree

3 files changed

+90
-15
lines changed

3 files changed

+90
-15
lines changed

packages/react-devtools-shared/src/backend/fiber/renderer.js

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
gt,
5151
gte,
5252
parseSourceFromComponentStack,
53+
parseSourceFromOwnerStack,
5354
serializeToString,
5455
} from 'react-devtools-shared/src/backend/utils';
5556
import {
@@ -5805,15 +5806,13 @@ export function attach(
58055806
function getSourceForFiberInstance(
58065807
fiberInstance: FiberInstance,
58075808
): Source | null {
5808-
const unresolvedSource = fiberInstance.source;
5809-
if (
5810-
unresolvedSource !== null &&
5811-
typeof unresolvedSource === 'object' &&
5812-
!isError(unresolvedSource)
5813-
) {
5814-
// $FlowFixMe: isError should have refined it.
5815-
return unresolvedSource;
5809+
// Favor the owner source if we have one.
5810+
const ownerSource = getSourceForInstance(fiberInstance);
5811+
if (ownerSource !== null) {
5812+
return ownerSource;
58165813
}
5814+
5815+
// Otherwise fallback to the throwing trick.
58175816
const dispatcherRef = getDispatcherRef(renderer);
58185817
const stackFrame =
58195818
dispatcherRef == null
@@ -5824,18 +5823,15 @@ export function attach(
58245823
dispatcherRef,
58255824
);
58265825
if (stackFrame === null) {
5827-
// If we don't find a source location by throwing, try to get one
5828-
// from an owned child if possible. This is the same branch as
5829-
// for virtual instances.
5830-
return getSourceForInstance(fiberInstance);
5826+
return null;
58315827
}
58325828
const source = parseSourceFromComponentStack(stackFrame);
58335829
fiberInstance.source = source;
58345830
return source;
58355831
}
58365832

58375833
function getSourceForInstance(instance: DevToolsInstance): Source | null {
5838-
let unresolvedSource = instance.source;
5834+
const unresolvedSource = instance.source;
58395835
if (unresolvedSource === null) {
58405836
// We don't have any source yet. We can try again later in case an owned child mounts later.
58415837
// TODO: We won't have any information here if the child is filtered.
@@ -5848,7 +5844,9 @@ export function attach(
58485844
// any intermediate utility functions. This won't point to the top of the component function
58495845
// but it's at least somewhere within it.
58505846
if (isError(unresolvedSource)) {
5851-
unresolvedSource = formatOwnerStack((unresolvedSource: any));
5847+
return (instance.source = parseSourceFromOwnerStack(
5848+
(unresolvedSource: any),
5849+
));
58525850
}
58535851
if (typeof unresolvedSource === 'string') {
58545852
const idx = unresolvedSource.lastIndexOf('\n');

packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@ export function formatOwnerStack(error: Error): string {
1313
const prevPrepareStackTrace = Error.prepareStackTrace;
1414
// $FlowFixMe[incompatible-type] It does accept undefined.
1515
Error.prepareStackTrace = undefined;
16-
let stack = error.stack;
16+
const stack = error.stack;
1717
Error.prepareStackTrace = prevPrepareStackTrace;
18+
return formatOwnerStackString(stack);
19+
}
20+
21+
export function formatOwnerStackString(stack: string): string {
1822
if (stack.startsWith('Error: react-stack-top-frame\n')) {
1923
// V8's default formatting prefixes with the error message which we
2024
// don't want/need.

packages/react-devtools-shared/src/backend/utils/index.js

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import type {DehydratedData} from 'react-devtools-shared/src/frontend/types';
1818
export {default as formatWithStyles} from './formatWithStyles';
1919
export {default as formatConsoleArguments} from './formatConsoleArguments';
2020

21+
import {formatOwnerStackString} from '../shared/DevToolsOwnerStack';
22+
2123
// TODO: update this to the first React version that has a corresponding DevTools backend
2224
const FIRST_DEVTOOLS_BACKEND_LOCKSTEP_VER = '999.9.9';
2325
export function hasAssignedBackend(version?: string): boolean {
@@ -345,6 +347,77 @@ export function parseSourceFromComponentStack(
345347
return parseSourceFromFirefoxStack(componentStack);
346348
}
347349

350+
let collectedLocation: Source | null = null;
351+
352+
function collectStackTrace(
353+
error: Error,
354+
structuredStackTrace: CallSite[],
355+
): string {
356+
let result: null | Source = null;
357+
// Collect structured stack traces from the callsites.
358+
// We mirror how V8 serializes stack frames and how we later parse them.
359+
for (let i = 0; i < structuredStackTrace.length; i++) {
360+
const callSite = structuredStackTrace[i];
361+
if (callSite.getFunctionName() === 'react-stack-bottom-frame') {
362+
// We pick the last frame that matches before the bottom frame since
363+
// that will be immediately inside the component as opposed to some helper.
364+
// If we don't find a bottom frame then we bail to string parsing.
365+
collectedLocation = result;
366+
// Skip everything after the bottom frame since it'll be internals.
367+
break;
368+
} else {
369+
const sourceURL = callSite.getScriptNameOrSourceURL();
370+
const line =
371+
// $FlowFixMe[prop-missing]
372+
typeof callSite.getEnclosingLineNumber === 'function'
373+
? (callSite: any).getEnclosingLineNumber()
374+
: callSite.getLineNumber();
375+
const col =
376+
// $FlowFixMe[prop-missing]
377+
typeof callSite.getEnclosingColumnNumber === 'function'
378+
? (callSite: any).getEnclosingColumnNumber()
379+
: callSite.getLineNumber();
380+
if (!sourceURL || !line || !col) {
381+
// Skip eval etc. without source url. They don't have location.
382+
continue;
383+
}
384+
result = {
385+
sourceURL,
386+
line: line,
387+
column: col,
388+
};
389+
}
390+
}
391+
// At the same time we generate a string stack trace just in case someone
392+
// else reads it.
393+
const name = error.name || 'Error';
394+
const message = error.message || '';
395+
let stack = name + ': ' + message;
396+
for (let i = 0; i < structuredStackTrace.length; i++) {
397+
stack += '\n at ' + structuredStackTrace[i].toString();
398+
}
399+
return stack;
400+
}
401+
402+
export function parseSourceFromOwnerStack(error: Error): Source | null {
403+
// First attempt to collected the structured data using prepareStackTrace.
404+
collectedLocation = null;
405+
const previousPrepare = Error.prepareStackTrace;
406+
Error.prepareStackTrace = collectStackTrace;
407+
let stack;
408+
try {
409+
stack = error.stack;
410+
} finally {
411+
Error.prepareStackTrace = previousPrepare;
412+
}
413+
if (collectedLocation !== null) {
414+
return collectedLocation;
415+
}
416+
// Fallback to parsing the string form.
417+
const componentStack = formatOwnerStackString(stack);
418+
return parseSourceFromComponentStack(componentStack);
419+
}
420+
348421
// 0.123456789 => 0.123
349422
// Expects high-resolution timestamp in milliseconds, like from performance.now()
350423
// Mainly used for optimizing the size of serialized profiling payload

0 commit comments

Comments
 (0)