Skip to content

Commit

Permalink
always include source file content sources in HMR source maps
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vzaidman authored and facebook-github-bot committed Jan 27, 2025
1 parent d8bf840 commit cd7a935
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
10 changes: 8 additions & 2 deletions packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -50,7 +55,8 @@ function generateModules(
extension,
),
);
return url.format(options.clientUrl);
delete moduleUrl.query.excludeSource;
return url.format(moduleUrl);
};

const sourceMappingURL = getURL('map');
Expand Down
3 changes: 3 additions & 0 deletions packages/metro/src/HmrServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ class HmrServer<TClient: Client> {
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 = {
Expand Down

0 comments on commit cd7a935

Please sign in to comment.