Skip to content

Commit 3da2616

Browse files
authored
fix: path handling in react devtools (#29199)
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Fix how devtools handles URLs. It - cannot handle relative source map URLs `//# sourceMappingURL=x.map` - cannot recognize Windows style URLs ## How did you test this change? works on my side
1 parent 15ca8b6 commit 3da2616

File tree

5 files changed

+59
-26
lines changed

5 files changed

+59
-26
lines changed

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

+30
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
REACT_STRICT_MODE_TYPE as StrictMode,
2727
} from 'shared/ReactSymbols';
2828
import {createElement} from 'react';
29+
import {symbolicateSource} from '../symbolicateSource';
2930

3031
describe('utils', () => {
3132
describe('getDisplayName', () => {
@@ -385,6 +386,35 @@ describe('utils', () => {
385386
});
386387
});
387388

389+
describe('symbolicateSource', () => {
390+
const source = `"use strict";
391+
Object.defineProperty(exports, "__esModule", { value: true });
392+
exports.f = f;
393+
function f() { }
394+
//# sourceMappingURL=`;
395+
const result = {
396+
column: 16,
397+
line: 1,
398+
sourceURL: 'http://test/a.mts',
399+
};
400+
const fs = {
401+
'http://test/a.mts': `export function f() {}`,
402+
'http://test/a.mjs.map': `{"version":3,"file":"a.mjs","sourceRoot":"","sources":["a.mts"],"names":[],"mappings":";;AAAA,cAAsB;AAAtB,SAAgB,CAAC,KAAI,CAAC"}`,
403+
'http://test/a.mjs': `${source}a.mjs.map`,
404+
'http://test/b.mjs': `${source}./a.mjs.map`,
405+
'http://test/c.mjs': `${source}http://test/a.mjs.map`,
406+
'http://test/d.mjs': `${source}/a.mjs.map`,
407+
};
408+
const fetchFileWithCaching = async (url: string) => fs[url] || null;
409+
it('should parse source map urls', async () => {
410+
const run = url => symbolicateSource(fetchFileWithCaching, url, 4, 10);
411+
await expect(run('http://test/a.mjs')).resolves.toStrictEqual(result);
412+
await expect(run('http://test/b.mjs')).resolves.toStrictEqual(result);
413+
await expect(run('http://test/c.mjs')).resolves.toStrictEqual(result);
414+
await expect(run('http://test/d.mjs')).resolves.toStrictEqual(result);
415+
});
416+
});
417+
388418
describe('formatConsoleArguments', () => {
389419
it('works with empty arguments list', () => {
390420
expect(formatConsoleArguments(...[])).toEqual([]);

packages/react-devtools-shared/src/hooks/SourceMapConsumer.js

+4-11
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ type SearchPosition = {
2424
type ResultPosition = {
2525
column: number,
2626
line: number,
27-
sourceContent: string,
28-
sourceURL: string,
27+
sourceContent: string | null,
28+
sourceURL: string | null,
2929
};
3030

3131
export type SourceMapConsumerType = {
@@ -118,18 +118,11 @@ function BasicSourceMapConsumer(sourceMapJSON: BasicSourceMap) {
118118
const line = nearestEntry[2] + 1;
119119
const column = nearestEntry[3];
120120

121-
if (sourceContent === null || sourceURL === null) {
122-
// TODO maybe fall back to the runtime source instead of throwing?
123-
throw Error(
124-
`Could not find original source for line:${lineNumber} and column:${columnNumber}`,
125-
);
126-
}
127-
128121
return {
129122
column,
130123
line,
131-
sourceContent: ((sourceContent: any): string),
132-
sourceURL: ((sourceURL: any): string),
124+
sourceContent: ((sourceContent: any): string | null),
125+
sourceURL: ((sourceURL: any): string | null),
133126
};
134127
}
135128

packages/react-devtools-shared/src/hooks/parseHookNames/parseSourceAndMetadata.js

+5
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,11 @@ function parseSourceAST(
276276
columnNumber,
277277
lineNumber,
278278
});
279+
if (sourceContent === null || sourceURL === null) {
280+
throw Error(
281+
`Could not find original source for line:${lineNumber} and column:${columnNumber}`,
282+
);
283+
}
279284

280285
originalSourceColumnNumber = column;
281286
originalSourceLineNumber = line;

packages/react-devtools-shared/src/symbolicateSource.js

+19-14
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export async function symbolicateSourceWithCache(
3939
}
4040

4141
const SOURCE_MAP_ANNOTATION_PREFIX = 'sourceMappingURL=';
42-
async function symbolicateSource(
42+
export async function symbolicateSource(
4343
fetchFileWithCaching: FetchFileWithCaching,
4444
sourceURL: string,
4545
lineNumber: number, // 1-based
@@ -63,11 +63,12 @@ async function symbolicateSource(
6363
const sourceMapAnnotationStartIndex = resourceLine.indexOf(
6464
SOURCE_MAP_ANNOTATION_PREFIX,
6565
);
66-
const sourceMapURL = resourceLine.slice(
66+
const sourceMapAt = resourceLine.slice(
6767
sourceMapAnnotationStartIndex + SOURCE_MAP_ANNOTATION_PREFIX.length,
6868
resourceLine.length,
6969
);
7070

71+
const sourceMapURL = new URL(sourceMapAt, sourceURL).toString();
7172
const sourceMap = await fetchFileWithCaching(sourceMapURL).catch(
7273
() => null,
7374
);
@@ -84,29 +85,33 @@ async function symbolicateSource(
8485
columnNumber, // 1-based
8586
});
8687

88+
if (possiblyURL === null) {
89+
return null;
90+
}
8791
try {
88-
void new URL(possiblyURL); // This is a valid URL
92+
// sourceMapURL = https://react.dev/script.js.map
93+
void new URL(possiblyURL); // test if it is a valid URL
8994
const normalizedURL = normalizeUrl(possiblyURL);
9095

9196
return {sourceURL: normalizedURL, line, column};
9297
} catch (e) {
9398
// This is not valid URL
94-
if (possiblyURL.startsWith('/')) {
99+
if (
100+
// sourceMapURL = /file
101+
possiblyURL.startsWith('/') ||
102+
// sourceMapURL = C:\\...
103+
possiblyURL.slice(1).startsWith(':\\\\')
104+
) {
95105
// This is an absolute path
96106
return {sourceURL: possiblyURL, line, column};
97107
}
98108

99109
// This is a relative path
100-
const [sourceMapAbsolutePathWithoutQueryParameters] =
101-
sourceMapURL.split(/[?#&]/);
102-
103-
const absoluteSourcePath =
104-
sourceMapAbsolutePathWithoutQueryParameters +
105-
(sourceMapAbsolutePathWithoutQueryParameters.endsWith('/')
106-
? ''
107-
: '/') +
108-
possiblyURL;
109-
110+
// possiblyURL = x.js.map, sourceMapURL = https://react.dev/script.js.map
111+
const absoluteSourcePath = new URL(
112+
possiblyURL,
113+
sourceMapURL,
114+
).toString();
110115
return {sourceURL: absoluteSourcePath, line, column};
111116
}
112117
} catch (e) {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ export function backendToFrontendSerializedElementMapper(
10171017
};
10181018
}
10191019

1020-
// This is a hacky one to just support this exact case.
1020+
// Chrome normalizes urls like webpack-internals:// but new URL don't, so cannot use new URL here.
10211021
export function normalizeUrl(url: string): string {
10221022
return url.replace('/./', '/');
10231023
}

0 commit comments

Comments
 (0)