Skip to content

Commit

Permalink
Emit JSC-safe URLs in HMR, //# sourceURL, Content-Location (faceb…
Browse files Browse the repository at this point in the history 
…ook#989)

Summary:
Pull Request resolved: facebook#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: b13c712521f7a3b6716c14d392cb3a057ede5936
  • Loading branch information
@robhogan @facebook-github-bot
robhogan authored and facebook-github-bot committed May 24, 2023
1 parent e148bcc commit 2e29fc7
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 17 deletions.
5 changes: 4 additions & 1 deletion packages/metro/src/DeltaBundler/Serializers/helpers/js.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -59,7 +60,9 @@ function getModuleParams(module: Module<>, options: Options): Array<mixed> {
// 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');

Expand Down
3 changes: 2 additions & 1 deletion packages/metro/src/DeltaBundler/Serializers/hmrJSBundle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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` +
Expand Down
5 changes: 4 additions & 1 deletion packages/metro/src/Server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
18 changes: 13 additions & 5 deletions packages/metro/src/Server/__tests__/Server-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
);
},
Expand All @@ -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'),
);
});
Expand All @@ -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) =>
Expand Down Expand Up @@ -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'),
);
});
Expand All @@ -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'),
);
});
Expand Down Expand Up @@ -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'),
);
},
Expand Down
16 changes: 8 additions & 8 deletions packages/metro/src/__tests__/HmrServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')],
Expand Down Expand Up @@ -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',
},
Expand Down Expand Up @@ -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')],
Expand Down Expand Up @@ -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')],
Expand Down Expand Up @@ -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')],
Expand Down
3 changes: 2 additions & 1 deletion packages/metro/src/lib/parseOptionsFromUrl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
),
Expand Down

0 comments on commit 2e29fc7

Please sign in to comment.