Skip to content

Commit 32994c5

Browse files
robhoganrobhogan
committed
Emit JSC-safe URLs in HMR, //# sourceURL, Content-Location (#989)
Summary: Pull Request resolved: #989 The remaining Metro part of the implementation of react-native-community/discussions-and-proposals#646, to fix (along with an RN change): - facebook/react-native#36794 and - expo/expo#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
1 parent 8a4b6cb commit 32994c5

File tree

5 files changed

+29
-16
lines changed

5 files changed

+29
-16
lines changed

‎packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {DeltaResult, Graph, Module} from '../types.flow';
1515
import type {HmrModule} from 'metro-runtime/src/modules/types.flow';
1616

1717
const {isJsModule, wrapModule} = require('./helpers/js');
18+
const jscSafeUrl = require('jsc-safe-url');
1819
const {addParamsToDefineCall} = require('metro-transform-plugins');
1920
const path = require('path');
2021
const url = require('url');
@@ -52,7 +53,7 @@ function generateModules(
5253
};
5354

5455
const sourceMappingURL = getURL('map');
55-
const sourceURL = getURL('bundle');
56+
const sourceURL = jscSafeUrl.toJscSafeUrl(getURL('bundle'));
5657
const code =
5758
prepareModule(module, graph, options) +
5859
`\n//# sourceMappingURL=${sourceMappingURL}\n` +

‎packages/metro/src/Server.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,7 @@ class Server {
781781
bundle: bundleCode,
782782
};
783783
},
784-
finish({req, mres, result}) {
784+
finish({req, mres, serializerOptions, result}) {
785785
if (
786786
// We avoid parsing the dates since the client should never send a more
787787
// recent date than the one returned by the Delta Bundler (if that's the
@@ -798,6 +798,9 @@ class Server {
798798
String(result.numModifiedFiles),
799799
);
800800
mres.setHeader(DELTA_ID_HEADER, String(result.nextRevId));
801+
if (serializerOptions?.sourceUrl != null) {
802+
mres.setHeader('Content-Location', serializerOptions.sourceUrl);
803+
}
801804
mres.setHeader('Content-Type', 'application/javascript; charset=UTF-8');
802805
mres.setHeader('Last-Modified', result.lastModifiedDate.toUTCString());
803806
mres.setHeader(

‎packages/metro/src/Server/__tests__/Server-test.js‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ describe('processRequest', () => {
265265
'__d(function() {foo();},1,[],"foo.js");',
266266
'require(0);',
267267
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true',
268-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true',
268+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true',
269269
].join('\n'),
270270
);
271271
});
@@ -309,7 +309,7 @@ describe('processRequest', () => {
309309
'__d(function() {entry();},0,[1],"mybundle.js");',
310310
'__d(function() {foo();},1,[],"foo.js");',
311311
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=false',
312-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=false',
312+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=false',
313313
].join('\n'),
314314
);
315315
});
@@ -334,6 +334,14 @@ describe('processRequest', () => {
334334
});
335335
});
336336

337+
it('returns Content-Location header on request of *.bundle', () => {
338+
return makeRequest('mybundle.bundle?runModule=true').then(response => {
339+
expect(response.getHeader('Content-Location')).toEqual(
340+
'http://localhost:8081/mybundle.bundle//&runModule=true',
341+
);
342+
});
343+
});
344+
337345
it('returns 404 on request of *.bundle when resource does not exist', async () => {
338346
fs.realpath = jest.fn((file, cb) =>
339347
cb(new ResourceNotFoundError('unknown.bundle')),
@@ -410,7 +418,7 @@ describe('processRequest', () => {
410418
'__d(function() {entry();},0,[1],"mybundle.js");',
411419
'__d(function() {foo();},1,[],"foo.js");',
412420
'//# sourceMappingURL=//localhost:8081/mybundle.map?modulesOnly=true&runModule=false',
413-
'//# sourceURL=http://localhost:8081/mybundle.bundle?modulesOnly=true&runModule=false',
421+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&modulesOnly=true&runModule=false',
414422
].join('\n'),
415423
);
416424
});
@@ -425,7 +433,7 @@ describe('processRequest', () => {
425433
[
426434
'__d(function() {entry();},0,[1],"mybundle.js");',
427435
'//# sourceMappingURL=//localhost:8081/mybundle.map?shallow=true&modulesOnly=true&runModule=false',
428-
'//# sourceURL=http://localhost:8081/mybundle.bundle?shallow=true&modulesOnly=true&runModule=false',
436+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&shallow=true&modulesOnly=true&runModule=false',
429437
].join('\n'),
430438
);
431439
});
@@ -682,7 +690,7 @@ describe('processRequest', () => {
682690
'__d(function() {foo();},1,[],"foo.js");',
683691
'require(0);',
684692
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true&TEST_URL_WAS_REWRITTEN=true',
685-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true&TEST_URL_WAS_REWRITTEN=true',
693+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true&TEST_URL_WAS_REWRITTEN=true',
686694
].join('\n'),
687695
);
688696
},

‎packages/metro/src/__tests__/HmrServer-test.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -280,12 +280,12 @@ describe('HmrServer', () => {
280280
'/root/hi-id',
281281
'__d(function() { alert("hi"); },"/root/hi-id",[],"hi",{});\n' +
282282
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
283-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
283+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
284284
],
285285
sourceMappingURL:
286286
'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
287287
sourceURL:
288-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
288+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
289289
},
290290
],
291291
deleted: ['/root/bye-id'],
@@ -365,11 +365,11 @@ describe('HmrServer', () => {
365365
'/root/hi-id',
366366
'__d(function() { alert("hi"); },"/root/hi-id",[],"hi",{});\n' +
367367
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
368-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
368+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
369369
],
370370

371371
sourceURL:
372-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
372+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
373373
sourceMappingURL:
374374
'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
375375
},
@@ -424,10 +424,10 @@ describe('HmrServer', () => {
424424
'/root/hi-id',
425425
'__d(function() { alert("hi"); },"/root/hi-id",[],"hi",{});\n' +
426426
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
427-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
427+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
428428
],
429429
sourceURL:
430-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
430+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
431431
},
432432
],
433433
deleted: ['/root/bye-id'],
@@ -480,7 +480,7 @@ describe('HmrServer', () => {
480480
{
481481
module: expect.any(Array),
482482
sourceURL:
483-
'http://localhost/hi.bundle?platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
483+
'http://localhost/hi.bundle//&platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
484484
},
485485
],
486486
deleted: ['/root/bye-id'],
@@ -533,7 +533,7 @@ describe('HmrServer', () => {
533533
{
534534
module: expect.any(Array),
535535
sourceURL:
536-
'http://localhost/hi.bundle?platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
536+
'http://localhost/hi.bundle//&platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
537537
},
538538
],
539539
deleted: ['/root/bye-id'],

‎packages/metro/src/lib/parseOptionsFromUrl.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {BundleOptions} from '../shared/types.flow';
1414

1515
const parsePlatformFilePath = require('../node-haste/lib/parsePlatformFilePath');
1616
const parseCustomTransformOptions = require('./parseCustomTransformOptions');
17+
const jscSafeUrl = require('jsc-safe-url');
1718
const nullthrows = require('nullthrows');
1819
const path = require('path');
1920
const url = require('url');
@@ -101,7 +102,7 @@ module.exports = function parseOptionsFromUrl(
101102
platform != null && platform.match(/^(android|ios)$/) ? 'http' : '',
102103
pathname: pathname.replace(/\.(bundle|delta)$/, '.map'),
103104
}),
104-
sourceUrl: normalizedRequestUrl,
105+
sourceUrl: jscSafeUrl.toJscSafeUrl(normalizedRequestUrl),
105106
unstable_transformProfile: getTransformProfile(
106107
query.unstable_transformProfile,
107108
),

0 commit comments

Comments
 (0)