Skip to content

Commit 1314702

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 ff92b4b commit 1314702

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');
@@ -50,7 +51,7 @@ function generateModules(
5051
};
5152

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

‎packages/metro/src/Server.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ class Server {
842842
bundle: bundleCode,
843843
};
844844
},
845-
finish({req, mres, result}) {
845+
finish({req, mres, serializerOptions, result}) {
846846
if (
847847
// We avoid parsing the dates since the client should never send a more
848848
// recent date than the one returned by the Delta Bundler (if that's the
@@ -859,6 +859,9 @@ class Server {
859859
String(result.numModifiedFiles),
860860
);
861861
mres.setHeader(DELTA_ID_HEADER, String(result.nextRevId));
862+
if (serializerOptions?.sourceUrl != null) {
863+
mres.setHeader('Content-Location', serializerOptions.sourceUrl);
864+
}
862865
mres.setHeader('Content-Type', 'application/javascript; charset=UTF-8');
863866
mres.setHeader('Last-Modified', result.lastModifiedDate.toUTCString());
864867
mres.setHeader(

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ describe('processRequest', () => {
318318
'__d(function() {foo();},1,[],"foo.js");',
319319
'require(0);',
320320
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true',
321-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true',
321+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true',
322322
].join('\n'),
323323
);
324324
},
@@ -360,7 +360,7 @@ describe('processRequest', () => {
360360
'__d(function() {entry();},0,[1],"mybundle.js");',
361361
'__d(function() {foo();},1,[],"foo.js");',
362362
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=false',
363-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=false',
363+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=false',
364364
].join('\n'),
365365
);
366366
});
@@ -385,6 +385,14 @@ describe('processRequest', () => {
385385
});
386386
});
387387

388+
it('returns Content-Location header on request of *.bundle', () => {
389+
return makeRequest('mybundle.bundle?runModule=true').then(response => {
390+
expect(response.getHeader('Content-Location')).toEqual(
391+
'http://localhost:8081/mybundle.bundle//&runModule=true',
392+
);
393+
});
394+
});
395+
388396
it('returns 404 on request of *.bundle when resource does not exist', async () => {
389397
// $FlowFixMe[cannot-write]
390398
fs.realpath = jest.fn((file, cb) =>
@@ -464,7 +472,7 @@ describe('processRequest', () => {
464472
'__d(function() {entry();},0,[1],"mybundle.js");',
465473
'__d(function() {foo();},1,[],"foo.js");',
466474
'//# sourceMappingURL=//localhost:8081/mybundle.map?modulesOnly=true&runModule=false',
467-
'//# sourceURL=http://localhost:8081/mybundle.bundle?modulesOnly=true&runModule=false',
475+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&modulesOnly=true&runModule=false',
468476
].join('\n'),
469477
);
470478
});
@@ -479,7 +487,7 @@ describe('processRequest', () => {
479487
[
480488
'__d(function() {entry();},0,[1],"mybundle.js");',
481489
'//# sourceMappingURL=//localhost:8081/mybundle.map?shallow=true&modulesOnly=true&runModule=false',
482-
'//# sourceURL=http://localhost:8081/mybundle.bundle?shallow=true&modulesOnly=true&runModule=false',
490+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&shallow=true&modulesOnly=true&runModule=false',
483491
].join('\n'),
484492
);
485493
});
@@ -741,7 +749,7 @@ describe('processRequest', () => {
741749
'__d(function() {foo();},1,[],"foo.js");',
742750
'require(0);',
743751
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true&TEST_URL_WAS_REWRITTEN=true',
744-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true&TEST_URL_WAS_REWRITTEN=true',
752+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true&TEST_URL_WAS_REWRITTEN=true',
745753
].join('\n'),
746754
);
747755
},

‎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
@@ -16,6 +16,7 @@ import type {TransformProfile} from 'metro-babel-transformer';
1616
const parsePlatformFilePath = require('../node-haste/lib/parsePlatformFilePath');
1717
const parseCustomResolverOptions = require('./parseCustomResolverOptions');
1818
const parseCustomTransformOptions = require('./parseCustomTransformOptions');
19+
const jscSafeUrl = require('jsc-safe-url');
1920
const nullthrows = require('nullthrows');
2021
const path = require('path');
2122
const url = require('url');
@@ -92,7 +93,7 @@ module.exports = function parseOptionsFromUrl(
9293
platform != null && platform.match(/^(android|ios)$/) ? 'http' : '',
9394
pathname: pathname.replace(/\.(bundle|delta)$/, '.map'),
9495
}),
95-
sourceUrl: normalizedRequestUrl,
96+
sourceUrl: jscSafeUrl.toJscSafeUrl(normalizedRequestUrl),
9697
unstable_transformProfile: getTransformProfile(
9798
query.unstable_transformProfile,
9899
),

0 commit comments

Comments
 (0)