Repository: facebook/react
Pull request: fix: path handling in react devtools

<!--
  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

Changed files:
filename: packages/react-devtools-shared/src/__tests__/utils-test.js
patch: @@ -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', () => {
@@ -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([]);

filename: packages/react-devtools-shared/src/hooks/SourceMapConsumer.js
patch: @@ -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 = {
@@ -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),
     };
   }
 

filename: packages/react-devtools-shared/src/hooks/parseHookNames/parseSourceAndMetadata.js
patch: @@ -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;

filename: packages/react-devtools-shared/src/symbolicateSource.js
patch: @@ -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
@@ -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,
       );
@@ -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) {

filename: packages/react-devtools-shared/src/utils.js
patch: @@ -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('/./', '/');
 }