Skip to content

Commit 7513996

Browse files
sebmarkbagesebmarkbage
authored
[DevTools] Unify by using ReactFunctionLocation type instead of Source (facebook#33955)
In RSC and other stacks now we use a lot of `ReactFunctionLocation` type to represent the ___location of a function. I.e. the ___location of the beginning of the function (the enclosing line/col) that is represented by the "Source" of the function. This is also what the parent Component Stacks represents. As opposed to `ReactCallSite` which is what normal stack traces and owner stacks represent. I.e. the line/column number of the callsite into the next function. We can start sharing more code by using the `ReactFunctionLocation` type to represent the component source ___location and it also helps clarify which ones are function locations and which ones are callsites as we start adding more stack traces (e.g. for async debug info and owner stack traces).
1 parent bb4418d commit 7513996

File tree

19 files changed

+158
-167
lines changed

19 files changed

+158
-167
lines changed

‎packages/react-devtools-core/src/standalone.js

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
import {localStorageSetItem} from 'react-devtools-shared/src/storage';
2727

2828
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
29-
import type {Source} from 'react-devtools-shared/src/shared/types';
29+
import type {ReactFunctionLocation} from 'shared/ReactTypes';
3030

3131
export type StatusTypes = 'server-connected' | 'devtools-connected' | 'error';
3232
export type StatusListener = (message: string, status: StatusTypes) => void;
@@ -144,29 +144,27 @@ async function fetchFileWithCaching(url: string) {
144144
}
145145

146146
function canViewElementSourceFunction(
147-
_source: Source,
148-
symbolicatedSource: Source | null,
147+
_source: ReactFunctionLocation,
148+
symbolicatedSource: ReactFunctionLocation | null,
149149
): boolean {
150150
if (symbolicatedSource == null) {
151151
return false;
152152
}
153+
const [, sourceURL, ,] = symbolicatedSource;
153154

154-
return doesFilePathExist(symbolicatedSource.sourceURL, projectRoots);
155+
return doesFilePathExist(sourceURL, projectRoots);
155156
}
156157

157158
function viewElementSourceFunction(
158-
_source: Source,
159-
symbolicatedSource: Source | null,
159+
_source: ReactFunctionLocation,
160+
symbolicatedSource: ReactFunctionLocation | null,
160161
): void {
161162
if (symbolicatedSource == null) {
162163
return;
163164
}
164165

165-
launchEditor(
166-
symbolicatedSource.sourceURL,
167-
symbolicatedSource.line,
168-
projectRoots,
169-
);
166+
const [, sourceURL, line] = symbolicatedSource;
167+
launchEditor(sourceURL, line, projectRoots);
170168
}
171169

172170
function onDisconnected() {

‎packages/react-devtools-extensions/src/main/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ function createBridgeAndStore() {
124124
};
125125

126126
const viewElementSourceFunction = (source, symbolicatedSource) => {
127-
const {sourceURL, line, column} = symbolicatedSource
127+
const [, sourceURL, line, column] = symbolicatedSource
128128
? symbolicatedSource
129129
: source;
130130

‎packages/react-devtools-fusebox/src/frontend.d.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,23 @@ export type Config = {
2828
export function createBridge(wall: Wall): Bridge;
2929
export function createStore(bridge: Bridge, config?: Config): Store;
3030

31-
export type Source = {
32-
sourceURL: string,
33-
line: number,
34-
column: number,
35-
};
31+
export type ReactFunctionLocation = [
32+
string, // function name
33+
string, // file name TODO: model nested eval locations as nested arrays
34+
number, // enclosing line number
35+
number, // enclosing column number
36+
];
3637
export type ViewElementSource = (
37-
source: Source,
38-
symbolicatedSource: Source | null,
38+
source: ReactFunctionLocation,
39+
symbolicatedSource: ReactFunctionLocation | null,
3940
) => void;
4041
export type ViewAttributeSource = (
4142
id: number,
4243
path: Array<string | number>,
4344
) => void;
4445
export type CanViewElementSource = (
45-
source: Source,
46-
symbolicatedSource: Source | null,
46+
source: ReactFunctionLocation,
47+
symbolicatedSource: ReactFunctionLocation | null,
4748
) => boolean;
4849

4950
export type InitializationOptions = {

‎packages/react-devtools-shared/src/__tests__/utils-test.js

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
getDisplayNameForReactElement,
1313
isPlainObject,
1414
} from 'react-devtools-shared/src/utils';
15-
import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
15+
import {stackToComponentLocations} from 'react-devtools-shared/src/devtools/utils';
1616
import {
1717
formatConsoleArguments,
1818
formatConsoleArgumentsToSingleString,
@@ -63,14 +63,17 @@ describe('utils', () => {
6363

6464
it('should parse a component stack trace', () => {
6565
expect(
66-
stackToComponentSources(`
66+
stackToComponentLocations(`
6767
at Foobar (http://localhost:3000/static/js/bundle.js:103:74)
6868
at a
6969
at header
7070
at div
7171
at App`),
7272
).toEqual([
73-
['Foobar', ['http://localhost:3000/static/js/bundle.js', 103, 74]],
73+
[
74+
'Foobar',
75+
['Foobar', 'http://localhost:3000/static/js/bundle.js', 103, 74],
76+
],
7477
['a', null],
7578
['header', null],
7679
['div', null],
@@ -315,12 +318,12 @@ describe('utils', () => {
315318
'at f (https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8519)\n' +
316319
'at r (https://react.dev/_next/static/chunks/pages/_app-dd0b77ea7bd5b246.js:1:498)\n',
317320
),
318-
).toEqual({
319-
sourceURL:
320-
'https://react.dev/_next/static/chunks/main-78a3b4c2aa4e4850.js',
321-
line: 1,
322-
column: 10389,
323-
});
321+
).toEqual([
322+
'',
323+
'https://react.dev/_next/static/chunks/main-78a3b4c2aa4e4850.js',
324+
1,
325+
10389,
326+
]);
324327
});
325328

326329
it('should construct the source from highest available frame', () => {
@@ -338,12 +341,12 @@ describe('utils', () => {
338341
' at tt (https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js:1:165520)\n' +
339342
' at f (https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8519)',
340343
),
341-
).toEqual({
342-
sourceURL:
343-
'https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js',
344-
line: 5,
345-
column: 9236,
346-
});
344+
).toEqual([
345+
'',
346+
'https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js',
347+
5,
348+
9236,
349+
]);
347350
});
348351

349352
it('should construct the source from frame, which has only url specified', () => {
@@ -353,12 +356,12 @@ describe('utils', () => {
353356
' at a\n' +
354357
' at https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js:5:9236\n',
355358
),
356-
).toEqual({
357-
sourceURL:
358-
'https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js',
359-
line: 5,
360-
column: 9236,
361-
});
359+
).toEqual([
360+
'',
361+
'https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js',
362+
5,
363+
9236,
364+
]);
362365
});
363366

364367
it('should parse sourceURL correctly if it includes parentheses', () => {
@@ -368,12 +371,12 @@ describe('utils', () => {
368371
' at Router (webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/app-router.js:181:11)\n' +
369372
' at ErrorBoundaryHandler (webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/error-boundary.js:114:9)',
370373
),
371-
).toEqual({
372-
sourceURL:
373-
'webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/react-dev-overlay/hot-reloader-client.js',
374-
line: 307,
375-
column: 11,
376-
});
374+
).toEqual([
375+
'',
376+
'webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/react-dev-overlay/hot-reloader-client.js',
377+
307,
378+
11,
379+
]);
377380
});
378381

379382
it('should support Firefox stack', () => {
@@ -383,12 +386,12 @@ describe('utils', () => {
383386
'f@https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8535\n' +
384387
'r@https://react.dev/_next/static/chunks/pages/_app-dd0b77ea7bd5b246.js:1:513',
385388
),
386-
).toEqual({
387-
sourceURL:
388-
'https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js',
389-
line: 1,
390-
column: 165558,
391-
});
389+
).toEqual([
390+
'',
391+
'https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js',
392+
1,
393+
165558,
394+
]);
392395
});
393396
});
394397

@@ -398,11 +401,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
398401
exports.f = f;
399402
function f() { }
400403
//# sourceMappingURL=`;
401-
const result = {
402-
column: 16,
403-
line: 1,
404-
sourceURL: 'http://test/a.mts',
405-
};
404+
const result = ['', 'http://test/a.mts', 1, 16];
406405
const fs = {
407406
'http://test/a.mts': `export function f() {}`,
408407
'http://test/a.mjs.map': `{"version":3,"file":"a.mjs","sourceRoot":"","sources":["a.mts"],"names":[],"mappings":";;AAAA,cAAsB;AAAtB,SAAgB,CAAC,KAAI,CAAC"}`,

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ import type {
145145
ElementType,
146146
Plugins,
147147
} from 'react-devtools-shared/src/frontend/types';
148-
import type {Source} from 'react-devtools-shared/src/shared/types';
148+
import type {ReactFunctionLocation} from 'shared/ReactTypes';
149149
import {getSourceLocationByFiber} from './DevToolsFiberComponentStack';
150150
import {formatOwnerStack} from '../shared/DevToolsOwnerStack';
151151

@@ -162,7 +162,7 @@ type FiberInstance = {
162162
parent: null | DevToolsInstance,
163163
firstChild: null | DevToolsInstance,
164164
nextSibling: null | DevToolsInstance,
165-
source: null | string | Error | Source, // source ___location of this component function, or owned child stack
165+
source: null | string | Error | ReactFunctionLocation, // source ___location of this component function, or owned child stack
166166
logCount: number, // total number of errors/warnings last seen
167167
treeBaseDuration: number, // the profiled time of the last render of this subtree
168168
data: Fiber, // one of a Fiber pair
@@ -190,7 +190,7 @@ type FilteredFiberInstance = {
190190
parent: null | DevToolsInstance,
191191
firstChild: null | DevToolsInstance,
192192
nextSibling: null | DevToolsInstance,
193-
source: null | string | Error | Source, // always null here.
193+
source: null | string | Error | ReactFunctionLocation, // always null here.
194194
logCount: number, // total number of errors/warnings last seen
195195
treeBaseDuration: number, // the profiled time of the last render of this subtree
196196
data: Fiber, // one of a Fiber pair
@@ -222,7 +222,7 @@ type VirtualInstance = {
222222
parent: null | DevToolsInstance,
223223
firstChild: null | DevToolsInstance,
224224
nextSibling: null | DevToolsInstance,
225-
source: null | string | Error | Source, // source ___location of this server component, or owned child stack
225+
source: null | string | Error | ReactFunctionLocation, // source ___location of this server component, or owned child stack
226226
logCount: number, // total number of errors/warnings last seen
227227
treeBaseDuration: number, // the profiled time of the last render of this subtree
228228
// The latest info for this instance. This can be updated over time and the
@@ -5805,7 +5805,7 @@ export function attach(
58055805

58065806
function getSourceForFiberInstance(
58075807
fiberInstance: FiberInstance,
5808-
): Source | null {
5808+
): ReactFunctionLocation | null {
58095809
// Favor the owner source if we have one.
58105810
const ownerSource = getSourceForInstance(fiberInstance);
58115811
if (ownerSource !== null) {
@@ -5830,7 +5830,9 @@ export function attach(
58305830
return source;
58315831
}
58325832

5833-
function getSourceForInstance(instance: DevToolsInstance): Source | null {
5833+
function getSourceForInstance(
5834+
instance: DevToolsInstance,
5835+
): ReactFunctionLocation | null {
58345836
let unresolvedSource = instance.source;
58355837
if (unresolvedSource === null) {
58365838
// We don't have any source yet. We can try again later in case an owned child mounts later.

‎packages/react-devtools-shared/src/backend/types.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import type {
3232
import type {InitBackend} from 'react-devtools-shared/src/backend';
3333
import type {TimelineDataExport} from 'react-devtools-timeline/src/types';
3434
import type {BackendBridge} from 'react-devtools-shared/src/bridge';
35-
import type {Source} from 'react-devtools-shared/src/shared/types';
35+
import type {ReactFunctionLocation} from 'shared/ReactTypes';
3636
import type Agent from './agent';
3737

3838
type BundleType =
@@ -281,7 +281,7 @@ export type InspectedElement = {
281281

282282
// List of owners
283283
owners: Array<SerializedElement> | null,
284-
source: Source | null,
284+
source: ReactFunctionLocation | null,
285285

286286
type: ElementType,
287287

0 commit comments

Comments
 (0)