Skip to content

Commit

Permalink
Remove module name from asyncRequire calls in release builds
Browse files Browse the repository at this point in the history 
Summary:
Changelog:

**[Experimental]**: Reorder `asyncRequire`'s parameters and make module name optional

Changes the dependency collector in Metro to remove the `moduleName` parameter from `asyncRequire` calls in release builds. This aligns `asyncRequire` with `require` and other core runtime dependency APIs, where the module name parameter is passed last, and is only present for debugging purposes.

Reviewed By: jacdebug

Differential Revision: D42632385

fbshipit-source-id: a7e23e607b3a4bbd2c7933c884053d2c43f9af75
  • Loading branch information
@motiz88 @facebook-github-bot
motiz88 authored and facebook-github-bot committed Mar 16, 2023
1 parent 20c4a65 commit 4e5261c
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 26 deletions.
49 changes: 45 additions & 4 deletions packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,26 @@ it('collects asynchronous dependencies', () => {
]);
expect(codeFromAst(ast)).toEqual(
comparableCode(`
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], "some/async/module", _dependencyMap.paths).then(foo => {});
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], _dependencyMap.paths, "some/async/module").then(foo => {});
`),
);
});

it('collects asynchronous dependencies with keepRequireNames: false', () => {
const ast = astFromCode(`
import("some/async/module").then(foo => {});
`);
const {dependencies, dependencyMapName} = collectDependencies(ast, {
...opts,
keepRequireNames: false,
});
expect(dependencies).toEqual([
{name: 'some/async/module', data: objectContaining({asyncType: 'async'})},
{name: 'asyncRequire', data: objectContaining({asyncType: null})},
]);
expect(codeFromAst(ast)).toEqual(
comparableCode(`
require(${dependencyMapName}[1])(${dependencyMapName}[0], _dependencyMap.paths).then(foo => {});
`),
);
});
Expand All @@ -834,7 +853,7 @@ it('distinguishes sync and async dependencies on the same module', () => {
expect(codeFromAst(ast)).toEqual(
comparableCode(`
const a = require(${dependencyMapName}[0], "some/async/module");
require(${dependencyMapName}[2], "asyncRequire")(${dependencyMapName}[1], "some/async/module", _dependencyMap.paths).then(foo => {});
require(${dependencyMapName}[2], "asyncRequire")(${dependencyMapName}[1], _dependencyMap.paths, "some/async/module").then(foo => {});
`),
);
});
Expand All @@ -852,7 +871,7 @@ it('distinguishes sync and async dependencies on the same module; reverse order'
]);
expect(codeFromAst(ast)).toEqual(
comparableCode(`
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], "some/async/module", _dependencyMap.paths).then(foo => {});
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], _dependencyMap.paths, "some/async/module").then(foo => {});
const a = require(${dependencyMapName}[2], "some/async/module");
`),
);
Expand All @@ -873,7 +892,29 @@ describe('import() prefetching', () => {
]);
expect(codeFromAst(ast)).toEqual(
comparableCode(`
require(${dependencyMapName}[1], "asyncRequire").prefetch(${dependencyMapName}[0], "some/async/module", _dependencyMap.paths);
require(${dependencyMapName}[1], "asyncRequire").prefetch(${dependencyMapName}[0], _dependencyMap.paths, "some/async/module");
`),
);
});

it('keepRequireNames: false', () => {
const ast = astFromCode(`
__prefetchImport("some/async/module");
`);
const {dependencies, dependencyMapName} = collectDependencies(ast, {
...opts,
keepRequireNames: false,
});
expect(dependencies).toEqual([
{
name: 'some/async/module',
data: objectContaining({asyncType: 'prefetch'}),
},
{name: 'asyncRequire', data: objectContaining({asyncType: null})},
]);
expect(codeFromAst(ast)).toEqual(
comparableCode(`
require(${dependencyMapName}[1]).prefetch(${dependencyMapName}[0], _dependencyMap.paths);
`),
);
});
Expand Down
60 changes: 38 additions & 22 deletions packages/metro/src/ModuleGraph/worker/collectDependencies.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -626,11 +626,19 @@ const dynamicRequireErrorTemplate = template.expression(`
* "require(...)" call to the asyncRequire specified.
*/
const makeAsyncRequireTemplate = template.expression(`
require(ASYNC_REQUIRE_MODULE_PATH)(MODULE_ID, MODULE_NAME, DEPENDENCY_MAP.paths)
require(ASYNC_REQUIRE_MODULE_PATH)(MODULE_ID, DEPENDENCY_MAP.paths)
`);

const makeAsyncRequireTemplateWithName = template.expression(`
require(ASYNC_REQUIRE_MODULE_PATH)(MODULE_ID, DEPENDENCY_MAP.paths, MODULE_NAME)
`);

const makeAsyncPrefetchTemplate = template.expression(`
require(ASYNC_REQUIRE_MODULE_PATH).prefetch(MODULE_ID, MODULE_NAME, DEPENDENCY_MAP.paths)
require(ASYNC_REQUIRE_MODULE_PATH).prefetch(MODULE_ID, DEPENDENCY_MAP.paths)
`);

const makeAsyncPrefetchTemplateWithName = template.expression(`
require(ASYNC_REQUIRE_MODULE_PATH).prefetch(MODULE_ID, DEPENDENCY_MAP.paths, MODULE_NAME)
`);

const makeResolveWeakTemplate = template.expression(`
Expand Down Expand Up @@ -661,33 +669,41 @@ const DefaultDependencyTransformer: DependencyTransformer = {
dependency: InternalDependency,
state: State,
): void {
path.replaceWith(
makeAsyncRequireTemplate({
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
state.asyncRequireModulePathStringLiteral,
),
MODULE_ID: createModuleIDExpression(dependency, state),
MODULE_NAME: createModuleNameLiteral(dependency),
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
}),
);
const makeNode = state.keepRequireNames
? makeAsyncRequireTemplateWithName
: makeAsyncRequireTemplate;
const opts = {
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
state.asyncRequireModulePathStringLiteral,
),
MODULE_ID: createModuleIDExpression(dependency, state),
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
...(state.keepRequireNames
? {MODULE_NAME: createModuleNameLiteral(dependency)}
: null),
};
path.replaceWith(makeNode(opts));
},

transformPrefetch(
path: NodePath<>,
dependency: InternalDependency,
state: State,
): void {
path.replaceWith(
makeAsyncPrefetchTemplate({
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
state.asyncRequireModulePathStringLiteral,
),
MODULE_ID: createModuleIDExpression(dependency, state),
MODULE_NAME: createModuleNameLiteral(dependency),
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
}),
);
const makeNode = state.keepRequireNames
? makeAsyncPrefetchTemplateWithName
: makeAsyncPrefetchTemplate;
const opts = {
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
state.asyncRequireModulePathStringLiteral,
),
MODULE_ID: createModuleIDExpression(dependency, state),
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
...(state.keepRequireNames
? {MODULE_NAME: createModuleNameLiteral(dependency)}
: null),
};
path.replaceWith(makeNode(opts));
},

transformIllegalDynamicRequire(path: NodePath<>, state: State): void {
Expand Down

0 comments on commit 4e5261c

Please sign in to comment.