Skip to content

Commit

Permalink
fix: path handling in react devtools (#29199)
Browse files Browse the repository at this point in the history
<!--
  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
  • Loading branch information
Jack-Works committed Jul 4, 2024
1 parent 15ca8b6 commit 3da2616
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 26 deletions.
30 changes: 30 additions & 0 deletions packages/react-devtools-shared/src/__tests__/utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
REACT_STRICT_MODE_TYPE as StrictMode,
} from 'shared/ReactSymbols';
import {createElement} from 'react';
import {symbolicateSource} from '../symbolicateSource';

describe('utils', () => {
describe('getDisplayName', () => {
Expand Down Expand Up @@ -385,6 +386,35 @@ describe('utils', () => {
});
});

describe('symbolicateSource', () => {
const source = `"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.f = f;
function f() { }
//# sourceMappingURL=`;
const result = {
column: 16,
line: 1,
sourceURL: 'http://test/a.mts',
};
const fs = {
'http://test/a.mts': `export function f() {}`,
'http://test/a.mjs.map': `{"version":3,"file":"a.mjs","sourceRoot":"","sources":["a.mts"],"names":[],"mappings":";;AAAA,cAAsB;AAAtB,SAAgB,CAAC,KAAI,CAAC"}`,
'http://test/a.mjs': `${source}a.mjs.map`,
'http://test/b.mjs': `${source}./a.mjs.map`,
'http://test/c.mjs': `${source}http://test/a.mjs.map`,
'http://test/d.mjs': `${source}/a.mjs.map`,
};
const fetchFileWithCaching = async (url: string) => fs[url] || null;
it('should parse source map urls', async () => {
const run = url => symbolicateSource(fetchFileWithCaching, url, 4, 10);
await expect(run('http://test/a.mjs')).resolves.toStrictEqual(result);
await expect(run('http://test/b.mjs')).resolves.toStrictEqual(result);
await expect(run('http://test/c.mjs')).resolves.toStrictEqual(result);
await expect(run('http://test/d.mjs')).resolves.toStrictEqual(result);
});
});

describe('formatConsoleArguments', () => {
it('works with empty arguments list', () => {
expect(formatConsoleArguments(...[])).toEqual([]);
Expand Down
15 changes: 4 additions & 11 deletions packages/react-devtools-shared/src/hooks/SourceMapConsumer.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ type SearchPosition = {
type ResultPosition = {
column: number,
line: number,
sourceContent: string,
sourceURL: string,
sourceContent: string | null,
sourceURL: string | null,
};

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

if (sourceContent === null || sourceURL === null) {
// TODO maybe fall back to the runtime source instead of throwing?
throw Error(
`Could not find original source for line:${lineNumber} and column:${columnNumber}`,
);
}

return {
column,
line,
sourceContent: ((sourceContent: any): string),
sourceURL: ((sourceURL: any): string),
sourceContent: ((sourceContent: any): string | null),
sourceURL: ((sourceURL: any): string | null),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@ function parseSourceAST(
columnNumber,
lineNumber,
});
if (sourceContent === null || sourceURL === null) {
throw Error(
`Could not find original source for line:${lineNumber} and column:${columnNumber}`,
);
}

originalSourceColumnNumber = column;
originalSourceLineNumber = line;
Expand Down
33 changes: 19 additions & 14 deletions packages/react-devtools-shared/src/symbolicateSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export async function symbolicateSourceWithCache(
}

const SOURCE_MAP_ANNOTATION_PREFIX = 'sourceMappingURL=';
async function symbolicateSource(
export async function symbolicateSource(
fetchFileWithCaching: FetchFileWithCaching,
sourceURL: string,
lineNumber: number, // 1-based
Expand All @@ -63,11 +63,12 @@ async function symbolicateSource(
const sourceMapAnnotationStartIndex = resourceLine.indexOf(
SOURCE_MAP_ANNOTATION_PREFIX,
);
const sourceMapURL = resourceLine.slice(
const sourceMapAt = resourceLine.slice(
sourceMapAnnotationStartIndex + SOURCE_MAP_ANNOTATION_PREFIX.length,
resourceLine.length,
);

const sourceMapURL = new URL(sourceMapAt, sourceURL).toString();
const sourceMap = await fetchFileWithCaching(sourceMapURL).catch(
() => null,
);
Expand All @@ -84,29 +85,33 @@ async function symbolicateSource(
columnNumber, // 1-based
});

if (possiblyURL === null) {
return null;
}
try {
void new URL(possiblyURL); // This is a valid URL
// sourceMapURL = https://react.dev/script.js.map
void new URL(possiblyURL); // test if it is a valid URL
const normalizedURL = normalizeUrl(possiblyURL);

return {sourceURL: normalizedURL, line, column};
} catch (e) {
// This is not valid URL
if (possiblyURL.startsWith('/')) {
if (
// sourceMapURL = /file
possiblyURL.startsWith('/') ||
// sourceMapURL = C:\\...
possiblyURL.slice(1).startsWith(':\\\\')
) {
// This is an absolute path
return {sourceURL: possiblyURL, line, column};
}

// This is a relative path
const [sourceMapAbsolutePathWithoutQueryParameters] =
sourceMapURL.split(/[?#&]/);

const absoluteSourcePath =
sourceMapAbsolutePathWithoutQueryParameters +
(sourceMapAbsolutePathWithoutQueryParameters.endsWith('/')
? ''
: '/') +
possiblyURL;

// possiblyURL = x.js.map, sourceMapURL = https://react.dev/script.js.map
const absoluteSourcePath = new URL(
possiblyURL,
sourceMapURL,
).toString();
return {sourceURL: absoluteSourcePath, line, column};
}
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ export function backendToFrontendSerializedElementMapper(
};
}

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

0 comments on commit 3da2616

Please sign in to comment.