Skip to content

Commit bc3dee9

Browse files
robhoganfacebook-github-bot
robhogan
authored and
facebook-github-bot
committed
Emit JSC-safe URLs in HMR, //# sourceURL, Content-Location
Summary: 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 ``` Differential Revision: D45983876 fbshipit-source-id: cbf749bae0256bc981588fa2b9e7b0d60eba7461
1 parent 795190a commit bc3dee9

File tree

6 files changed

+33
-17
lines changed

6 files changed

+33
-17
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {MixedOutput, Module} from '../../types.flow';
1515
import type {JsOutput} from 'metro-transform-worker';
1616

1717
const invariant = require('invariant');
18+
const jscSafeUrl = require('jsc-safe-url');
1819
const {addParamsToDefineCall} = require('metro-transform-plugins');
1920
const path = require('path');
2021

@@ -59,7 +60,9 @@ function getModuleParams(module: Module<>, options: Options): Array<mixed> {
5960
// Construct a server-relative URL for the split bundle, propagating
6061
// most parameters from the main bundle's URL.
6162

62-
const {searchParams} = new URL(options.sourceUrl);
63+
const {searchParams} = new URL(
64+
jscSafeUrl.toNormalUrl(options.sourceUrl),
65+
);
6366
searchParams.set('modulesOnly', 'true');
6467
searchParams.set('runModule', 'false');
6568

‎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
@@ -334,7 +334,7 @@ describe('processRequest', () => {
334334
'__d(function() {foo();},1,[],"foo.js");',
335335
'require(0);',
336336
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true',
337-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true',
337+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true',
338338
].join('\n'),
339339
);
340340
},
@@ -349,7 +349,7 @@ describe('processRequest', () => {
349349
'__d(function() {entry();},0,[1],"mybundle.js");',
350350
'__d(function() {foo();},1,[],"foo.js");',
351351
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=false',
352-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=false',
352+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=false',
353353
].join('\n'),
354354
);
355355
});
@@ -374,6 +374,14 @@ describe('processRequest', () => {
374374
});
375375
});
376376

377+
it('returns Content-Location header on request of *.bundle', () => {
378+
return makeRequest('mybundle.bundle?runModule=true').then(response => {
379+
expect(response.getHeader('Content-Location')).toEqual(
380+
'http://localhost:8081/mybundle.bundle//&runModule=true',
381+
);
382+
});
383+
});
384+
377385
it('returns 404 on request of *.bundle when resource does not exist', async () => {
378386
// $FlowFixMe[cannot-write]
379387
fs.realpath = jest.fn((file, cb: $FlowFixMe) =>
@@ -451,7 +459,7 @@ describe('processRequest', () => {
451459
'__d(function() {entry();},0,[1],"mybundle.js");',
452460
'__d(function() {foo();},1,[],"foo.js");',
453461
'//# sourceMappingURL=//localhost:8081/mybundle.map?modulesOnly=true&runModule=false',
454-
'//# sourceURL=http://localhost:8081/mybundle.bundle?modulesOnly=true&runModule=false',
462+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&modulesOnly=true&runModule=false',
455463
].join('\n'),
456464
);
457465
});
@@ -466,7 +474,7 @@ describe('processRequest', () => {
466474
[
467475
'__d(function() {entry();},0,[1],"mybundle.js");',
468476
'//# sourceMappingURL=//localhost:8081/mybundle.map?shallow=true&modulesOnly=true&runModule=false',
469-
'//# sourceURL=http://localhost:8081/mybundle.bundle?shallow=true&modulesOnly=true&runModule=false',
477+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&shallow=true&modulesOnly=true&runModule=false',
470478
].join('\n'),
471479
);
472480
});
@@ -730,7 +738,7 @@ describe('processRequest', () => {
730738
'__d(function() {foo();},1,[],"foo.js");',
731739
'require(0);',
732740
'//# sourceMappingURL=//localhost:8081/mybundle.map?runModule=true&TEST_URL_WAS_REWRITTEN=true',
733-
'//# sourceURL=http://localhost:8081/mybundle.bundle?runModule=true&TEST_URL_WAS_REWRITTEN=true',
741+
'//# sourceURL=http://localhost:8081/mybundle.bundle//&runModule=true&TEST_URL_WAS_REWRITTEN=true',
734742
].join('\n'),
735743
);
736744
},

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,12 @@ describe('HmrServer', () => {
339339
id('/root/hi') +
340340
',[],"hi",{});\n' +
341341
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
342-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
342+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
343343
],
344344
sourceMappingURL:
345345
'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
346346
sourceURL:
347-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
347+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
348348
},
349349
],
350350
deleted: [id('/root/bye')],
@@ -423,11 +423,11 @@ describe('HmrServer', () => {
423423
id('/root/hi') +
424424
',[],"hi",{});\n' +
425425
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
426-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
426+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
427427
],
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
sourceMappingURL:
432432
'http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
433433
},
@@ -482,10 +482,10 @@ describe('HmrServer', () => {
482482
id('/root/hi') +
483483
',[],"hi",{});\n' +
484484
'//# sourceMappingURL=http://localhost/hi.map?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n' +
485-
'//# sourceURL=http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
485+
'//# sourceURL=http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true\n',
486486
],
487487
sourceURL:
488-
'http://localhost/hi.bundle?platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
488+
'http://localhost/hi.bundle//&platform=ios&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
489489
},
490490
],
491491
deleted: [id('/root/bye')],
@@ -536,7 +536,7 @@ describe('HmrServer', () => {
536536
{
537537
module: expect.any(Array),
538538
sourceURL:
539-
'http://localhost/hi.bundle?platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
539+
'http://localhost/hi.bundle//&platform=ios&unusedExtraParam=42&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
540540
},
541541
],
542542
deleted: [id('/root/bye')],
@@ -587,7 +587,7 @@ describe('HmrServer', () => {
587587
{
588588
module: expect.any(Array),
589589
sourceURL:
590-
'http://localhost/hi.bundle?platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
590+
'http://localhost/hi.bundle//&platform=ios&TEST_URL_WAS_REWRITTEN=true&dev=true&minify=false&modulesOnly=true&runModule=false&shallow=true',
591591
},
592592
],
593593
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');
@@ -77,7 +78,7 @@ module.exports = function parseOptionsFromUrl(
7778
platform != null && platform.match(/^(android|ios)$/) ? 'http' : '',
7879
pathname: pathname.replace(/\.(bundle|delta)$/, '.map'),
7980
}),
80-
sourceUrl: normalizedRequestUrl,
81+
sourceUrl: jscSafeUrl.toJscSafeUrl(normalizedRequestUrl),
8182
unstable_transformProfile: getTransformProfile(
8283
query.unstable_transformProfile,
8384
),

0 commit comments

Comments
 (0)