Skip to content

Commit

Permalink
experimentalImportBundleSupport: Move bundle path hints into serialis…
Browse files Browse the repository at this point in the history
…ed dependency map

Summary:
Changelog: [Experimental]

## Problem

`experimentalImportBundleSupport` (D15943150 (72329d0)) originally relied on including a global map from module IDs to paths (`importBundleNames`) in each bundle served by the HTTP server. This allowed a custom (and Meta-internal) implementation of `asyncRequire` to construct and fetch an HTTP URL corresponding to any given async-loadable module.

The global map approach breaks Fast Refresh when new async imports are added to a module, because it turns out we never update the global map during HMR. New `import()` call sites can be added and are transformed to `asyncRequire`, but the map entries they need aren't there, so they just throw at runtime.

## Solution

Naively, fixing this would require teaching the entire HMR pipeline (Graph, DeltaCalculator, HmrServer, HmrClient) to incrementally update the `importBundleNames` map at runtime on the client.

Here we take a different approach: breaking up the `importBundleNames` map into individual maps, one for each module that contains async dependencies.

1. Every call to `asyncRequire` now has an extra argument `paths` which holds a *local* map from IDs to paths. Conceptually, this is the same information that `importBundleNames` has for the entire graph, except now it's split by "parent" module.
2. `paths` is generated by the serializer for async dependencies (only) and written to the bundle in a backwards-compatible format as part of the dependency map - which is now either an array or an *object with a numeric indexer*, depending on whether it has a `paths` property.

The HMR machinery already knows how to update dependency maps, so in particular it keeps `paths` up-to-date as part of that. No special plumbing needed! 🎉

## Next steps

* `graph.importBundleNames` will be going away in a separate diff; anyone using the (still experimental) `experimentalImportBundleSupport` API will need to update their `asyncRequire` implementation at that point to use this new mechanism instead (cc EvanBacon).
* Custom bundle splitting implementations may want something other than the *path* of a module, specifically, to be passed to `asyncRequire`. The contract is between the serializer and the `asyncRequire` implementation. We might want to use the term "fetch key" instead of "path", allow it to be any serializable value rather than just a string, and add a `createModuleFetchKey` API (analogous to `createModuleIdFactory`) to control this in config.

Differential Revision: D40412241

fbshipit-source-id: 622c63c770d6aa95fca90decebe2f38feb2c31b6
  • Loading branch information
motiz88 authored and facebook-github-bot committed Nov 30, 2022
1 parent 4942d56 commit 05d7264
Show file tree
Hide file tree
Showing 18 changed files with 262 additions and 192 deletions.
12 changes: 11 additions & 1 deletion packages/metro-runtime/src/polyfills/require.js
Expand Up @@ -17,7 +17,17 @@
declare var __DEV__: boolean;
declare var __METRO_GLOBAL_PREFIX__: string;

type DependencyMap = Array<ModuleID>;
// A simpler $ArrayLike<T>. Not iterable and doesn't have a `length`.
// This is compatible with actual arrays as well as with objects that look like
// {0: 'value', 1: '...'}
type ArrayIndexable<T> = interface {
+[indexer: number]: T,
};
type DependencyMap = $ReadOnly<
ArrayIndexable<ModuleID> & {
paths?: {[id: ModuleID]: string},
},
>;
type Exports = any;
type FactoryFn = (
global: Object,
Expand Down
Expand Up @@ -297,9 +297,11 @@ it('adds lazy imports at the end of a bundle', () => {
['/root/foo', fooModule],
['/root/bar', barModule],
]),

entryPoints: ['foo'],
importBundleNames: new Set(['/path/to/async/module.js']),
},

{
asyncRequireModulePath: '/asyncRequire.js',
processModuleFilter: () => true,
Expand All @@ -325,8 +327,7 @@ it('adds lazy imports at the end of a bundle', () => {
"__d(function() {/* code for bar */},\\"bar\\",[],\\"bar\\");",
],
],
"post": "(function(){var $$=require(\\"asyncRequire.js\\");$$.addImportBundleNames({\\"module.js\\":\\"../path/to/async/module\\"})})();
require(\\"foo\\");
"post": "require(\\"foo\\");
//# sourceMappingURL=http://localhost/bundle.map",
"pre": "__d(function() {/* code for polyfill */});",
}
Expand All @@ -343,9 +344,11 @@ it('lazy imports are relative to serverRoot if it differs from projectRoot', ()
['/root/foo', fooModule],
['/root/bar', barModule],
]),

entryPoints: ['foo'],
importBundleNames: new Set(['/path/to/async/module.js']),
},

{
asyncRequireModulePath: '/asyncRequire.js',
processModuleFilter: () => true,
Expand All @@ -371,8 +374,7 @@ it('lazy imports are relative to serverRoot if it differs from projectRoot', ()
"__d(function() {/* code for bar */},\\"bar\\",[],\\"bar\\");",
],
],
"post": "(function(){var $$=require(\\"asyncRequire.js\\");$$.addImportBundleNames({\\"module.js\\":\\"path/to/async/module\\"})})();
require(\\"foo\\");
"post": "require(\\"foo\\");
//# sourceMappingURL=http://localhost/bundle.map",
"pre": "__d(function() {/* code for polyfill */});",
}
Expand Down
29 changes: 12 additions & 17 deletions packages/metro/src/DeltaBundler/Serializers/baseBytecodeBundle.js
Expand Up @@ -37,7 +37,9 @@ function baseBytecodeBundle(
filter: options.processModuleFilter,
createModuleId: options.createModuleId,
dev: options.dev,
includeAsyncPaths: options.includeAsyncPaths,
projectRoot: options.projectRoot,
serverRoot: options.serverRoot,
};

// Do not prepend polyfills or the require runtime when only modules are requested
Expand All @@ -53,23 +55,16 @@ function baseBytecodeBundle(
const {compile} = require('metro-hermes-compiler');

const post = processBytecodeModules(
getAppendScripts(
entryPoint,
[...preModules, ...modules],
graph.importBundleNames,
{
asyncRequireModulePath: options.asyncRequireModulePath,
createModuleId: options.createModuleId,
getRunModuleStatement: options.getRunModuleStatement,
inlineSourceMap: options.inlineSourceMap,
projectRoot: options.projectRoot,
runBeforeMainModule: options.runBeforeMainModule,
runModule: options.runModule,
serverRoot: options.serverRoot,
sourceMapUrl: options.sourceMapUrl,
sourceUrl: options.sourceUrl,
},
).map(module => {
getAppendScripts(entryPoint, [...preModules, ...modules], {
asyncRequireModulePath: options.asyncRequireModulePath,
createModuleId: options.createModuleId,
getRunModuleStatement: options.getRunModuleStatement,
inlineSourceMap: options.inlineSourceMap,
runBeforeMainModule: options.runBeforeMainModule,
runModule: options.runModule,
sourceMapUrl: options.sourceMapUrl,
sourceUrl: options.sourceUrl,
}).map(module => {
return {
...module,
output: [
Expand Down
29 changes: 12 additions & 17 deletions packages/metro/src/DeltaBundler/Serializers/baseJSBundle.js
Expand Up @@ -36,7 +36,9 @@ function baseJSBundle(
filter: options.processModuleFilter,
createModuleId: options.createModuleId,
dev: options.dev,
includeAsyncPaths: options.includeAsyncPaths,
projectRoot: options.projectRoot,
serverRoot: options.serverRoot,
};

// Do not prepend polyfills or the require runtime when only modules are requested
Expand All @@ -54,23 +56,16 @@ function baseJSBundle(
);

const postCode = processModules(
getAppendScripts(
entryPoint,
[...preModules, ...modules],
graph.importBundleNames,
{
asyncRequireModulePath: options.asyncRequireModulePath,
createModuleId: options.createModuleId,
getRunModuleStatement: options.getRunModuleStatement,
inlineSourceMap: options.inlineSourceMap,
projectRoot: options.projectRoot,
runBeforeMainModule: options.runBeforeMainModule,
runModule: options.runModule,
serverRoot: options.serverRoot,
sourceMapUrl: options.sourceMapUrl,
sourceUrl: options.sourceUrl,
},
),
getAppendScripts(entryPoint, [...preModules, ...modules], {
asyncRequireModulePath: options.asyncRequireModulePath,
createModuleId: options.createModuleId,
getRunModuleStatement: options.getRunModuleStatement,
inlineSourceMap: options.inlineSourceMap,
runBeforeMainModule: options.runBeforeMainModule,
runModule: options.runModule,
sourceMapUrl: options.sourceMapUrl,
sourceUrl: options.sourceUrl,
}),
processModulesOptions,
)
.map(([_, code]) => code)
Expand Down
14 changes: 6 additions & 8 deletions packages/metro/src/DeltaBundler/Serializers/getRamBundleInfo.js
Expand Up @@ -26,12 +26,12 @@ const {sourceMapObject} = require('./sourceMapObject');
const nullthrows = require('nullthrows');
const path = require('path');

type Options = {
type Options = $ReadOnly<{
...SerializerOptions,
+excludeSource: boolean,
+getTransformOptions: ?GetTransformOptions,
+platform: ?string,
};
excludeSource: boolean,
getTransformOptions: ?GetTransformOptions,
platform: ?string,
}>;

export type RamBundleInfo = {
getDependencies: string => Set<string>,
Expand All @@ -50,9 +50,7 @@ async function getRamBundleInfo(
...pre,
...graph.dependencies.values(),
];
modules = modules.concat(
getAppendScripts(entryPoint, modules, graph.importBundleNames, options),
);
modules = modules.concat(getAppendScripts(entryPoint, modules, options));

modules.forEach((module: Module<>) => options.createModuleId(module.path));

Expand Down
Expand Up @@ -9,14 +9,13 @@
* @oncall react_native
*/

'use strict';
import type {Dependency} from '../../../types.flow';

import CountingSet from '../../../../lib/CountingSet';

const createModuleIdFactory = require('../../../../lib/createModuleIdFactory');
const {wrapModule} = require('../bytecode');
const {compile, validateBytecodeModule} = require('metro-hermes-compiler');
import createModuleIdFactory from '../../../../lib/createModuleIdFactory';
import {wrapModule} from '../bytecode';
import {compile, validateBytecodeModule} from 'metro-hermes-compiler';

let myModule, bytecode;

Expand Down Expand Up @@ -70,7 +69,9 @@ it('produces a bytecode header buffer for each module', () => {
const buffers = wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: true,
includeAsyncPaths: false,
projectRoot: '/root',
serverRoot: '/root',
});
expect(buffers.length).toBe(2);
expect(() => validateBytecodeModule(buffers[0], 0)).not.toThrow();
Expand All @@ -83,7 +84,9 @@ it('does not produce a bytecode header buffer for a script', () => {
const buffers = wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: true,
includeAsyncPaths: false,
projectRoot: '/root',
serverRoot: '/root',
});
expect(buffers.length).toBe(1);
expect(buffers[0]).toBe(bytecode);
Expand Down
Expand Up @@ -9,31 +9,34 @@
* @oncall react_native
*/

'use strict';
import type {Dependency} from '../../../types.flow';

import CountingSet from '../../../../lib/CountingSet';

const createModuleIdFactory = require('../../../../lib/createModuleIdFactory');
const {wrapModule} = require('../js');
import {wrap as raw} from 'jest-snapshot-serializer-raw';
import createModuleIdFactory from '../../../../lib/createModuleIdFactory';
import {wrapModule} from '../js';
import nullthrows from 'nullthrows';

let myModule;

expect.addSnapshotSerializer(require('jest-snapshot-serializer-raw'));

beforeEach(() => {
myModule = {
path: '/root/foo.js',
dependencies: new Map<string, Dependency>([
[
'bar',
{
absolutePath: '/bar',
absolutePath: '/bar.js',
data: {data: {asyncType: null, locs: [], key: 'bar'}, name: 'bar'},
},
],
[
'baz',
{
absolutePath: '/baz',
absolutePath: '/baz.js',
data: {data: {asyncType: null, locs: [], key: 'baz'}, name: 'baz'},
},
],
Expand All @@ -57,46 +60,106 @@ beforeEach(() => {
describe('wrapModule()', () => {
it('Should wrap a module in nondev mode', () => {
expect(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: false,
projectRoot: '/root',
}),
).toEqual('__d(function() { console.log("foo") },0,[1,2]);');
raw(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: false,
includeAsyncPaths: false,
projectRoot: '/root',
serverRoot: '/root',
}),
),
).toMatchInlineSnapshot(`__d(function() { console.log("foo") },0,[1,2]);`);
});

it('Should wrap a module in dev mode', () => {
expect(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: true,
projectRoot: '/root',
}),
).toEqual('__d(function() { console.log("foo") },0,[1,2],"foo.js");');
raw(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: true,
includeAsyncPaths: false,
projectRoot: '/root',
serverRoot: '/root',
}),
),
).toMatchInlineSnapshot(
`__d(function() { console.log("foo") },0,[1,2],"foo.js");`,
);
});

it('should not wrap a script', () => {
myModule.output[0].type = 'js/script';

expect(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: true,
projectRoot: '/root',
}),
).toEqual(myModule.output[0].data.code);
raw(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: true,
includeAsyncPaths: false,
projectRoot: '/root',
serverRoot: '/root',
}),
),
).toMatchInlineSnapshot(`__d(function() { console.log("foo") });`);
});

it('should use custom createModuleId param', () => {
// Just use a createModuleId that returns the same path.
expect(
wrapModule(myModule, {
createModuleId: (path: string) => path,
dev: false,
projectRoot: '/root',
}),
).toEqual(
'__d(function() { console.log("foo") },"/root/foo.js",["/bar","/baz"]);',
raw(
wrapModule(myModule, {
createModuleId: (path: string) => path,
dev: false,
includeAsyncPaths: false,
projectRoot: '/root',
serverRoot: '/root',
}),
),
).toMatchInlineSnapshot(
`__d(function() { console.log("foo") },"/root/foo.js",["/bar.js","/baz.js"]);`,
);
});

it('includes the paths of async dependencies when requested', () => {
const dep = nullthrows(myModule.dependencies.get('bar'));
myModule.dependencies.set('bar', {
...dep,
data: {...dep.data, data: {...dep.data.data, asyncType: 'async'}},
});
expect(
raw(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: false,
includeAsyncPaths: true,
projectRoot: '/root',
serverRoot: '/root',
}),
),
).toMatchInlineSnapshot(
`__d(function() { console.log("foo") },0,{"0":1,"1":2,"paths":{"1":"../bar"}});`,
);
});

it('async dependency paths respect serverRoot', () => {
const dep = nullthrows(myModule.dependencies.get('bar'));
myModule.dependencies.set('bar', {
...dep,
data: {...dep.data, data: {...dep.data.data, asyncType: 'async'}},
});
expect(
raw(
wrapModule(myModule, {
createModuleId: createModuleIdFactory(),
dev: false,
includeAsyncPaths: true,
projectRoot: '/root',
serverRoot: '/',
}),
),
).toMatchInlineSnapshot(
`__d(function() { console.log("foo") },0,{"0":1,"1":2,"paths":{"1":"bar"}});`,
);
});
});

0 comments on commit 05d7264

Please sign in to comment.