From bce6b27ef8ac7c41e0a3e990eb71747cc0e6f606 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 --- .../src/DeltaBundler/Serializers/helpers/js.js | 5 ++++- .../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 ++- 6 files changed, 33 insertions(+), 17 deletions(-) diff --git a/packages/metro/src/DeltaBundler/Serializers/helpers/js.js b/packages/metro/src/DeltaBundler/Serializers/helpers/js.js index c358356046..660e66c926 100644 --- a/packages/metro/src/DeltaBundler/Serializers/helpers/js.js +++ b/packages/metro/src/DeltaBundler/Serializers/helpers/js.js @@ -15,6 +15,7 @@ import type {MixedOutput, Module} from '../../types.flow'; import type {JsOutput} from 'metro-transform-worker'; const invariant = require('invariant'); +const jscSafeUrl = require('jsc-safe-url'); const {addParamsToDefineCall} = require('metro-transform-plugins'); const path = require('path'); @@ -59,7 +60,9 @@ function getModuleParams(module: Module<>, options: Options): Array { // Construct a server-relative URL for the split bundle, propagating // most parameters from the main bundle's URL. - const {searchParams} = new URL(options.sourceUrl); + const {searchParams} = new URL( + jscSafeUrl.toNormalUrl(options.sourceUrl), + ); searchParams.set('modulesOnly', 'true'); searchParams.set('runModule', 'false'); diff --git a/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js b/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js index 95e602f80a..0e1d45666b 100644 --- a/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js +++ b/packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js @@ -16,6 +16,7 @@ import type {DeltaResult, Module, ReadOnlyGraph} 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'); @@ -53,7 +54,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 0c294168ec..59fb4884cd 100644 --- a/packages/metro/src/Server.js +++ b/packages/metro/src/Server.js @@ -906,7 +906,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 @@ -923,6 +923,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 644b4655eb..5b63fd0e5d 100644 --- a/packages/metro/src/Server/__tests__/Server-test.js +++ b/packages/metro/src/Server/__tests__/Server-test.js @@ -334,7 +334,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'), ); }, @@ -349,7 +349,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'), ); }); @@ -374,6 +374,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: $FlowFixMe) => @@ -451,7 +459,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'), ); }); @@ -466,7 +474,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'), ); }); @@ -730,7 +738,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 e22a61551d..6228adeca8 100644 --- a/packages/metro/src/__tests__/HmrServer-test.js +++ b/packages/metro/src/__tests__/HmrServer-test.js @@ -339,12 +339,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')], @@ -423,11 +423,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', }, @@ -482,10 +482,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')], @@ -536,7 +536,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')], @@ -587,7 +587,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 11ffa9b3ef..a6ad03ff3a 100644 --- a/packages/metro/src/lib/parseOptionsFromUrl.js +++ b/packages/metro/src/lib/parseOptionsFromUrl.js @@ -17,6 +17,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'); @@ -77,7 +78,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, ),