Skip to content

Commit

Permalink
Emit JSC-safe URLs in HMR, //# sourceURL, Content-Location (#989)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@robhogan
robhogan committed Jun 5, 2023
1 parent 8a4b6cb commit 32994c5
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 16 deletions.
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 @@ -15,6 +15,7 @@ import type {DeltaResult, Graph, Module} 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 @@ -52,7 +53,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 @@ -781,7 +781,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 @@ -798,6 +798,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 @@ -265,7 +265,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 Down Expand Up @@ -309,7 +309,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 @@ -334,6 +334,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 () => {
fs.realpath = jest.fn((file, cb) =>
cb(new ResourceNotFoundError('unknown.bundle')),
Expand Down Expand Up @@ -410,7 +418,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 @@ -425,7 +433,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 @@ -682,7 +690,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 @@ -280,12 +280,12 @@ describe('HmrServer', () => {
'/root/hi-id',
'__d(function() { alert("hi"); },"/root/hi-id",[],"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: ['/root/bye-id'],
Expand Down Expand Up @@ -365,11 +365,11 @@ describe('HmrServer', () => {
'/root/hi-id',
'__d(function() { alert("hi"); },"/root/hi-id",[],"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 @@ -424,10 +424,10 @@ describe('HmrServer', () => {
'/root/hi-id',
'__d(function() { alert("hi"); },"/root/hi-id",[],"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: ['/root/bye-id'],
Expand Down Expand Up @@ -480,7 +480,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: ['/root/bye-id'],
Expand Down Expand Up @@ -533,7 +533,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: ['/root/bye-id'],
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 @@ -14,6 +14,7 @@ import type {BundleOptions} from '../shared/types.flow';

const parsePlatformFilePath = require('../node-haste/lib/parsePlatformFilePath');
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 @@ -101,7 +102,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 32994c5

Please sign in to comment.