Skip to content

Commit 8088bc2

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 cf36f42 commit 8088bc2

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
@@ -16,6 +16,7 @@ import type {DeltaResult, Module, ReadOnlyGraph} from '../types.flow';
1616
import type {HmrModule} from 'metro-runtime/src/modules/types.flow';
1717

1818
const {isJsModule, wrapModule} = require('./helpers/js');
19+
const jscSafeUrl = require('jsc-safe-url');
1920
const {addParamsToDefineCall} = require('metro-transform-plugins');
2021
const path = require('path');
2122
const url = require('url');
@@ -53,7 +54,7 @@ function generateModules(
5354
};
5455

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

‎packages/metro/src/Server.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ class Server {
906906
bundle: bundleCode,
907907
};
908908
},
909-
finish({req, mres, result}) {
909+
finish({req, mres, serializerOptions, result}) {
910910
if (
911911
// We avoid parsing the dates since the client should never send a more
912912
// recent date than the one returned by the Delta Bundler (if that's the
@@ -923,6 +923,9 @@ class Server {
923923
String(result.numModifiedFiles),
924924
);
925925
mres.setHeader(DELTA_ID_HEADER, String(result.nextRevId));
926+
if (serializerOptions?.sourceUrl != null) {
927+
mres.setHeader('Content-Location', serializerOptions.sourceUrl);
928+
}
926929
mres.setHeader('Content-Type', 'application/javascript; charset=UTF-8');
927930
mres.setHeader('Last-Modified', result.lastModifiedDate.toUTCString());
928931
mres.setHeader(

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ describe('processRequest', () => {
341341
'__d(function() {foo();},1,[],"foo.js");',
342342
'require(0);',
343343
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true',
344-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true',
344+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true',
345345
].join('\n'),
346346
);
347347
},
@@ -383,7 +383,7 @@ describe('processRequest', () => {
383383
'__d(function() {entry();},0,[1],"mybundle.js");',
384384
'__d(function() {foo();},1,[],"foo.js");',
385385
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=false',
386-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=false',
386+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=false',
387387
].join('\n'),
388388
);
389389
});
@@ -408,6 +408,14 @@ describe('processRequest', () => {
408408
});
409409
});
410410

411+
it('returns Content-Location header on request of *.bundle', () => {
412+
return makeRequest('mybundle.bundle?runModule=true').then(response => {
413+
expect(response.getHeader('Content-Location')).toEqual(
414+
'http://localhost:8081/mybundle.bundle//&runModule=true',
415+
);
416+
});
417+
});
418+
411419
it('returns 404 on request of *.bundle when resource does not exist', async () => {
412420
// $FlowFixMe[cannot-write]
413421
fs.realpath = jest.fn((file, cb: $FlowFixMe) =>
@@ -487,7 +495,7 @@ describe('processRequest', () => {
487495
'__d(function() {entry();},0,[1],"mybundle.js");',
488496
'__d(function() {foo();},1,[],"foo.js");',
489497
'//# sourceMappingURL=//localhost:8081/mybundle.map?modulesOnly=true&runModule=false',
490-
'//# sourceURL=http://localhost:8081/mybundle.bundle?modulesOnly=true&runModule=false',
498+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&modulesOnly=true&runModule=false',
491499
].join('\n'),
492500
);
493501
});
@@ -502,7 +510,7 @@ describe('processRequest', () => {
502510
[
503511
'__d(function() {entry();},0,[1],"mybundle.js");',
504512
'//# sourceMappingURL=//localhost:8081/mybundle.map?shallow=true&modulesOnly=true&runModule=false',
505-
'//# sourceURL=http://localhost:8081/mybundle.bundle?shallow=true&modulesOnly=true&runModule=false',
513+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&shallow=true&modulesOnly=true&runModule=false',
506514
].join('\n'),
507515
);
508516
});
@@ -764,7 +772,7 @@ describe('processRequest', () => {
764772
'__d(function() {foo();},1,[],"foo.js");',
765773
'require(0);',
766774
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true&TEST_URL_WAS_REWRITTEN=true',
767-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true&TEST_URL_WAS_REWRITTEN=true',
775+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true&TEST_URL_WAS_REWRITTEN=true',
768776
].join('\n'),
769777
);
770778
},

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,12 @@ describe('HmrServer', () => {
341341
id('/root/hi') +
342342
',[],"hi",{});\n' +
343343
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
344-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
344+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
345345
],
346346
sourceMappingURL:
347347
'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
348348
sourceURL:
349-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
349+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
350350
},
351351
],
352352
deleted: [id('/root/bye')],
@@ -425,11 +425,11 @@ describe('HmrServer', () => {
425425
id('/root/hi') +
426426
',[],"hi",{});\n' +
427427
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
428-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
428+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
429429
],
430430

431431
sourceURL:
432-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
432+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
433433
sourceMappingURL:
434434
'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
435435
},
@@ -484,10 +484,10 @@ describe('HmrServer', () => {
484484
id('/root/hi') +
485485
',[],"hi",{});\n' +
486486
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
487-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
487+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
488488
],
489489
sourceURL:
490-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
490+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
491491
},
492492
],
493493
deleted: [id('/root/bye')],
@@ -538,7 +538,7 @@ describe('HmrServer', () => {
538538
{
539539
module: expect.any(Array),
540540
sourceURL:
541-
'http://localhost/hi.bundle?platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
541+
'http://localhost/hi.bundle//&platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
542542
},
543543
],
544544
deleted: [id('/root/bye')],
@@ -589,7 +589,7 @@ describe('HmrServer', () => {
589589
{
590590
module: expect.any(Array),
591591
sourceURL:
592-
'http://localhost/hi.bundle?platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
592+
'http://localhost/hi.bundle//&platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
593593
},
594594
],
595595
deleted: [id('/root/bye')],

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {TransformProfile} from 'metro-babel-transformer';
1717
const parsePlatformFilePath = require('../node-haste/lib/parsePlatformFilePath');
1818
const parseCustomResolverOptions = require('./parseCustomResolverOptions');
1919
const parseCustomTransformOptions = require('./parseCustomTransformOptions');
20+
const jscSafeUrl = require('jsc-safe-url');
2021
const nullthrows = require('nullthrows');
2122
const path = require('path');
2223
const url = require('url');
@@ -93,7 +94,7 @@ module.exports = function parseOptionsFromUrl(
9394
platform != null && platform.match(/^(android|ios)$/) ? 'http' : '',
9495
pathname: pathname.replace(/\.(bundle|delta)$/, '.map'),
9596
}),
96-
sourceUrl: normalizedRequestUrl,
97+
sourceUrl: jscSafeUrl.toJscSafeUrl(normalizedRequestUrl),
9798
unstable_transformProfile: getTransformProfile(
9899
query.unstable_transformProfile,
99100
),

0 commit comments

Comments
 (0)