From d78ccebb5bc09dbc1f45542538fdd4bf6820e991 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Wed, 24 May 2023 14:48:19 -0700 Subject: [PATCH] Emit JSC-safe URLs in HMR, `//# sourceURL`, `Content-Location` (#989) Summary: Pull Request resolved: https://github.com/facebook/metro/pull/989 The remaining Metro part of the implementation of https://github.com/react-native-community/discussions-and-proposals/pull/646, to fix (along with an RN change): - https://github.com/facebook/react-native/issues/36794 and - https://github.com/expo/expo/issues/22008 This ensures that Metro always and consistently emits "JSC-safe" (i.e., `//&` query-delimited) URLs where they are used as a "source URL" for evaluated JS. This includes: - `sourceURL` within the JSON HMR payload (`HmrModule`) - `//# sourceURL` comments within the body of a base or HMR bundle - The new `Content-Location` header delivered in response to an HTTP bundle request. Clients will be expected to use these as source URL arguments to JS engines, in preference to the URL on which they might have connected/requested the bundle originally. ``` * **[Fix]**: Emit source URLs in a format that will not be stripped by JavaScriptCore ``` Reviewed By: GijsWeterings Differential Revision: D45983876 fbshipit-source-id: 3e7f0118091424b9c1b1d40e4eb7baeb5be1f48f --- .../DeltaBundler/Serializers/hmrJSBundle.js | 3 ++- packages/metro/src/Server.js | 5 ++++- .../metro/src/Server/__tests__/Server-test.js | 18 +++++++++++++----- packages/metro/src/__tests__/HmrServer-test.js | 16 ++++++++-------- packages/metro/src/lib/parseOptionsFromUrl.js | 3 ++- 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js b/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js index 50a2f53504..589ba615ce 100644 --- a/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js +++ b/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js @@ -15,6 +15,7 @@ import type {DeltaResult, Graph, Module} from '../types.flow'; import type {HmrModule} from 'metro-runtime/src/modules/types.flow'; const {isJsModule, wrapModule} = require('./helpers/js'); +const jscSafeUrl = require('jsc-safe-url'); const {addParamsToDefineCall} = require('metro-transform-plugins'); const path = require('path'); const url = require('url'); @@ -50,7 +51,7 @@ function generateModules( }; const sourceMappingURL = getURL('map'); - const sourceURL = getURL('bundle'); + const sourceURL = jscSafeUrl.toJscSafeUrl(getURL('bundle')); const code = prepareModule(module, graph, options) + `\n//# sourceMappingURL=${sourceMappingURL}\n` + diff --git a/packages/metro/src/Server.js b/packages/metro/src/Server.js index 1f8e1987a6..9e75c75954 100644 --- a/packages/metro/src/Server.js +++ b/packages/metro/src/Server.js @@ -842,7 +842,7 @@ class Server { bundle: bundleCode, }; }, - finish({req, mres, result}) { + finish({req, mres, serializerOptions, result}) { if ( // We avoid parsing the dates since the client should never send a more // recent date than the one returned by the Delta Bundler (if that's the @@ -859,6 +859,9 @@ class Server { String(result.numModifiedFiles), ); mres.setHeader(DELTA_ID_HEADER, String(result.nextRevId)); + if (serializerOptions?.sourceUrl != null) { + mres.setHeader('Content-Location', serializerOptions.sourceUrl); + } mres.setHeader('Content-Type', 'application/javascript; charset=UTF-8'); mres.setHeader('Last-Modified', result.lastModifiedDate.toUTCString()); mres.setHeader( diff --git a/packages/metro/src/Server/__tests__/Server-test.js b/packages/metro/src/Server/__tests__/Server-test.js index 09e2938adb..75f4127566 100644 --- a/packages/metro/src/Server/__tests__/Server-test.js +++ b/packages/metro/src/Server/__tests__/Server-test.js @@ -318,7 +318,7 @@ describe('processRequest', () => { '__d(function() {foo();},1,[],"foo.js");', 'require(0);', '//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true', - '//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true', + '//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true', ].join('\n'), ); }, @@ -360,7 +360,7 @@ describe('processRequest', () => { '__d(function() {entry();},0,[1],"mybundle.js");', '__d(function() {foo();},1,[],"foo.js");', '//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=false', - '//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=false', + '//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=false', ].join('\n'), ); }); @@ -385,6 +385,14 @@ describe('processRequest', () => { }); }); + it('returns Content-Location header on request of *.bundle', () => { + return makeRequest('mybundle.bundle?runModule=true').then(response => { + expect(response.getHeader('Content-Location')).toEqual( + 'http://localhost:8081/mybundle.bundle//&runModule=true', + ); + }); + }); + it('returns 404 on request of *.bundle when resource does not exist', async () => { // $FlowFixMe[cannot-write] fs.realpath = jest.fn((file, cb) => @@ -464,7 +472,7 @@ describe('processRequest', () => { '__d(function() {entry();},0,[1],"mybundle.js");', '__d(function() {foo();},1,[],"foo.js");', '//# sourceMappingURL=//localhost:8081/mybundle.map?modulesOnly=true&runModule=false', - '//# sourceURL=http://localhost:8081/mybundle.bundle?modulesOnly=true&runModule=false', + '//# sourceURL=http://localhost:8081/mybundle.bundle//&modulesOnly=true&runModule=false', ].join('\n'), ); }); @@ -479,7 +487,7 @@ describe('processRequest', () => { [ '__d(function() {entry();},0,[1],"mybundle.js");', '//# sourceMappingURL=//localhost:8081/mybundle.map?shallow=true&modulesOnly=true&runModule=false', - '//# sourceURL=http://localhost:8081/mybundle.bundle?shallow=true&modulesOnly=true&runModule=false', + '//# sourceURL=http://localhost:8081/mybundle.bundle//&shallow=true&modulesOnly=true&runModule=false', ].join('\n'), ); }); @@ -741,7 +749,7 @@ describe('processRequest', () => { '__d(function() {foo();},1,[],"foo.js");', 'require(0);', '//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true&TEST_URL_WAS_REWRITTEN=true', - '//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true&TEST_URL_WAS_REWRITTEN=true', + '//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true&TEST_URL_WAS_REWRITTEN=true', ].join('\n'), ); }, diff --git a/packages/metro/src/__tests__/HmrServer-test.js b/packages/metro/src/__tests__/HmrServer-test.js index a19c225bbb..7039de38cc 100644 --- a/packages/metro/src/__tests__/HmrServer-test.js +++ b/packages/metro/src/__tests__/HmrServer-test.js @@ -341,12 +341,12 @@ describe('HmrServer', () => { id('/root/hi') + ',[],"hi",{});\n' + '//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' + - '//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n', + '//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n', ], sourceMappingURL: 'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', sourceURL: - 'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', + 'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', }, ], deleted: [id('/root/bye')], @@ -425,11 +425,11 @@ describe('HmrServer', () => { id('/root/hi') + ',[],"hi",{});\n' + '//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' + - '//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n', + '//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n', ], sourceURL: - 'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', + 'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', sourceMappingURL: 'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', }, @@ -484,10 +484,10 @@ describe('HmrServer', () => { id('/root/hi') + ',[],"hi",{});\n' + '//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' + - '//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n', + '//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n', ], sourceURL: - 'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', + 'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', }, ], deleted: [id('/root/bye')], @@ -538,7 +538,7 @@ describe('HmrServer', () => { { module: expect.any(Array), sourceURL: - 'http://localhost/hi.bundle?platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', + 'http://localhost/hi.bundle//&platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', }, ], deleted: [id('/root/bye')], @@ -589,7 +589,7 @@ describe('HmrServer', () => { { module: expect.any(Array), sourceURL: - 'http://localhost/hi.bundle?platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', + 'http://localhost/hi.bundle//&platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true', }, ], deleted: [id('/root/bye')], diff --git a/packages/metro/src/lib/parseOptionsFromUrl.js b/packages/metro/src/lib/parseOptionsFromUrl.js index 0e2d50e025..ee5a9a2ce8 100644 --- a/packages/metro/src/lib/parseOptionsFromUrl.js +++ b/packages/metro/src/lib/parseOptionsFromUrl.js @@ -16,6 +16,7 @@ import type {TransformProfile} from 'metro-babel-transformer'; const parsePlatformFilePath = require('../node-haste/lib/parsePlatformFilePath'); const parseCustomResolverOptions = require('./parseCustomResolverOptions'); const parseCustomTransformOptions = require('./parseCustomTransformOptions'); +const jscSafeUrl = require('jsc-safe-url'); const nullthrows = require('nullthrows'); const path = require('path'); const url = require('url'); @@ -92,7 +93,7 @@ module.exports = function parseOptionsFromUrl( platform != null && platform.match(/^(android|ios)$/) ? 'http' : '', pathname: pathname.replace(/\.(bundle|delta)$/, '.map'), }), - sourceUrl: normalizedRequestUrl, + sourceUrl: jscSafeUrl.toJscSafeUrl(normalizedRequestUrl), unstable_transformProfile: getTransformProfile( query.unstable_transformProfile, ),