From cd7a93557453ff3c36e367ef7a049471f1fbe182 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Mon, 27 Jan 2025 07:37:01 -0800 Subject: [PATCH] always include source file content sources in HMR source maps Summary: A newly parsed source map for the same source code is either added to the list of source maps for that source file or replacing the old based on wether the source maps are considered to be "compatibleForURL" in the [`sourceMapAttached` function](https://github.com/facebookexperimental/rn-chrome-devtools-frontend/blob/ca9417cf91bf6a388eeb1915e4bd97fd9afe0a42/front_end/models/bindings/CompilerScriptMapping.ts#L425). Aside: How could several source maps be used for the same file? They can theoretically map different parts of the source file to different transpiled files/different parts of the same transpiled file. e.g. when a breakpoint is set, `Debugger.setBreakpointByUrl` is called for each source map, and only one of them actually sets a breakpoint (the others return an empty list of set breakpoints). In our case, all of them were resolved, because they were all mapping all the parts of the source file, creating duplicate breakpoints. Source maps are [considered to be compatible](https://github.com/facebookexperimental/rn-chrome-devtools-frontend/blob/main/front_end/core/sdk/SourceMap.ts#L821) if they agree on: * The existence of an `ignoreList` * The contents of `sourcesContent` that holds the contents of the original source file We were previous omitting `sourcesContent` from all source maps if `excludeSource` was used in the query that requested the source map. By omitting it, we essentially indicated to Chrome Dev-Tools that every hot reload is adding a new source map rather than replacing the previous one. Instead, we will populate `sourcesContent` on hot reload. Notice that it is still OK to omit `sourcesContent` from the initial large source map for the whole bundle, because the subsequent source maps for any of the source files it bundles will not be empty, meaning they will have a different `sourcesContent` from the empty one in the bundle's source maps, thus they will be considered "not compatible". Changelog: [Feature] always include source file content sources in HMR source maps Reviewed By: hezi, hoxyq Differential Revision: D68631591 fbshipit-source-id: d48726d0b6d9a419b25903a956395e9c77f2ac80 --- .../metro/src/DeltaBundler/Serializers/hmrJSBundle.js | 10 ++++++++-- packages/metro/src/HmrServer.js | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js b/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js index 070ee5141e..fc9593df01 100644 --- a/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js +++ b/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js @@ -41,7 +41,12 @@ function generateModules( if (isJsModule(module)) { // Construct a bundle URL for this specific module only const getURL = (extension: 'bundle' | 'map') => { - options.clientUrl.pathname = path.relative( + const moduleUrl = url.parse(url.format(options.clientUrl), true); + // the legacy url object is parsed with both "search" and "query" fields. + // for the "query" field to be used when formatting the object bach to string, the "search" field must be empty. + // https://nodejs.org/api/url.html#urlformaturlobject:~:text=If%20the%20urlObject.search%20property%20is%20undefined + moduleUrl.search = ''; + moduleUrl.pathname = path.relative( options.serverRoot ?? options.projectRoot, path.join( path.dirname(module.path), @@ -50,7 +55,8 @@ function generateModules( extension, ), ); - return url.format(options.clientUrl); + delete moduleUrl.query.excludeSource; + return url.format(moduleUrl); }; const sourceMappingURL = getURL('map'); diff --git a/packages/metro/src/HmrServer.js b/packages/metro/src/HmrServer.js index 6c7d7c1b48..3a12acfa8d 100644 --- a/packages/metro/src/HmrServer.js +++ b/packages/metro/src/HmrServer.js @@ -169,6 +169,9 @@ class HmrServer { runModule: runModule || 'false', shallow: 'true', }; + // the legacy url object is parsed with both "search" and "query" fields. + // for the "query" field to be used when formatting the object bach to string, the "search" field must be empty. + // https://nodejs.org/api/url.html#urlformaturlobject:~:text=If%20the%20urlObject.search%20property%20is%20undefined clientUrl.search = ''; clientGroup = {