From 32994c56ac9a1dbf2caaf98968073fa6cb89f20e 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 71534ebde..73a40b71b 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'); @@ -52,7 +53,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 d94b49e7a..4356e589a 100644 --- a/packages/metro/src/Server.js +++ b/packages/metro/src/Server.js @@ -781,7 +781,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 @@ -798,6 +798,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 da882a15e..e8201b762 100644 --- a/packages/metro/src/Server/__tests__/Server-test.js +++ b/packages/metro/src/Server/__tests__/Server-test.js @@ -265,7 +265,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'), ); }); @@ -309,7 +309,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'), ); }); @@ -334,6 +334,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 () => { fs.realpath = jest.fn((file, cb) => cb(new ResourceNotFoundError('unknown.bundle')), @@ -410,7 +418,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'), ); }); @@ -425,7 +433,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'), ); }); @@ -682,7 +690,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 5253a8bf3..6136bc978 100644 --- a/packages/metro/src/__tests__/HmrServer-test.js +++ b/packages/metro/src/__tests__/HmrServer-test.js @@ -280,12 +280,12 @@ describe('HmrServer', () => { '/root/hi-id', '__d(function() { alert("hi"); },"/root/hi-id",[],"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: ['/root/bye-id'], @@ -365,11 +365,11 @@ describe('HmrServer', () => { '/root/hi-id', '__d(function() { alert("hi"); },"/root/hi-id",[],"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', }, @@ -424,10 +424,10 @@ describe('HmrServer', () => { '/root/hi-id', '__d(function() { alert("hi"); },"/root/hi-id",[],"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: ['/root/bye-id'], @@ -480,7 +480,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: ['/root/bye-id'], @@ -533,7 +533,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: ['/root/bye-id'], diff --git a/packages/metro/src/lib/parseOptionsFromUrl.js b/packages/metro/src/lib/parseOptionsFromUrl.js index ef86b6a58..9e5630bca 100644 --- a/packages/metro/src/lib/parseOptionsFromUrl.js +++ b/packages/metro/src/lib/parseOptionsFromUrl.js @@ -14,6 +14,7 @@ import type {BundleOptions} from '../shared/types.flow'; const parsePlatformFilePath = require('../node-haste/lib/parsePlatformFilePath'); const parseCustomTransformOptions = require('./parseCustomTransformOptions'); +const jscSafeUrl = require('jsc-safe-url'); const nullthrows = require('nullthrows'); const path = require('path'); const url = require('url'); @@ -101,7 +102,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, ),