Skip to content

Commit 4e5261c

Browse files
motiz88facebook-github-bot
authored andcommitted
Remove module name from asyncRequire calls in release builds
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
1 parent 20c4a65 commit 4e5261c

File tree

2 files changed

+83
-26
lines changed

2 files changed

+83
-26
lines changed

packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js

+45-4
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,26 @@ it('collects asynchronous dependencies', () => {
815815
]);
816816
expect(codeFromAst(ast)).toEqual(
817817
comparableCode(`
818-
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], "some/async/module", _dependencyMap.paths).then(foo => {});
818+
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], _dependencyMap.paths, "some/async/module").then(foo => {});
819+
`),
820+
);
821+
});
822+
823+
it('collects asynchronous dependencies with keepRequireNames: false', () => {
824+
const ast = astFromCode(`
825+
import("some/async/module").then(foo => {});
826+
`);
827+
const {dependencies, dependencyMapName} = collectDependencies(ast, {
828+
...opts,
829+
keepRequireNames: false,
830+
});
831+
expect(dependencies).toEqual([
832+
{name: 'some/async/module', data: objectContaining({asyncType: 'async'})},
833+
{name: 'asyncRequire', data: objectContaining({asyncType: null})},
834+
]);
835+
expect(codeFromAst(ast)).toEqual(
836+
comparableCode(`
837+
require(${dependencyMapName}[1])(${dependencyMapName}[0], _dependencyMap.paths).then(foo => {});
819838
`),
820839
);
821840
});
@@ -834,7 +853,7 @@ it('distinguishes sync and async dependencies on the same module', () => {
834853
expect(codeFromAst(ast)).toEqual(
835854
comparableCode(`
836855
const a = require(${dependencyMapName}[0], "some/async/module");
837-
require(${dependencyMapName}[2], "asyncRequire")(${dependencyMapName}[1], "some/async/module", _dependencyMap.paths).then(foo => {});
856+
require(${dependencyMapName}[2], "asyncRequire")(${dependencyMapName}[1], _dependencyMap.paths, "some/async/module").then(foo => {});
838857
`),
839858
);
840859
});
@@ -852,7 +871,7 @@ it('distinguishes sync and async dependencies on the same module; reverse order'
852871
]);
853872
expect(codeFromAst(ast)).toEqual(
854873
comparableCode(`
855-
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], "some/async/module", _dependencyMap.paths).then(foo => {});
874+
require(${dependencyMapName}[1], "asyncRequire")(${dependencyMapName}[0], _dependencyMap.paths, "some/async/module").then(foo => {});
856875
const a = require(${dependencyMapName}[2], "some/async/module");
857876
`),
858877
);
@@ -873,7 +892,29 @@ describe('import() prefetching', () => {
873892
]);
874893
expect(codeFromAst(ast)).toEqual(
875894
comparableCode(`
876-
require(${dependencyMapName}[1], "asyncRequire").prefetch(${dependencyMapName}[0], "some/async/module", _dependencyMap.paths);
895+
require(${dependencyMapName}[1], "asyncRequire").prefetch(${dependencyMapName}[0], _dependencyMap.paths, "some/async/module");
896+
`),
897+
);
898+
});
899+
900+
it('keepRequireNames: false', () => {
901+
const ast = astFromCode(`
902+
__prefetchImport("some/async/module");
903+
`);
904+
const {dependencies, dependencyMapName} = collectDependencies(ast, {
905+
...opts,
906+
keepRequireNames: false,
907+
});
908+
expect(dependencies).toEqual([
909+
{
910+
name: 'some/async/module',
911+
data: objectContaining({asyncType: 'prefetch'}),
912+
},
913+
{name: 'asyncRequire', data: objectContaining({asyncType: null})},
914+
]);
915+
expect(codeFromAst(ast)).toEqual(
916+
comparableCode(`
917+
require(${dependencyMapName}[1]).prefetch(${dependencyMapName}[0], _dependencyMap.paths);
877918
`),
878919
);
879920
});

packages/metro/src/ModuleGraph/worker/collectDependencies.js

+38-22
Original file line numberDiff line numberDiff line change
@@ -626,11 +626,19 @@ const dynamicRequireErrorTemplate = template.expression(`
626626
* "require(...)" call to the asyncRequire specified.
627627
*/
628628
const makeAsyncRequireTemplate = template.expression(`
629-
require(ASYNC_REQUIRE_MODULE_PATH)(MODULE_ID, MODULE_NAME, DEPENDENCY_MAP.paths)
629+
require(ASYNC_REQUIRE_MODULE_PATH)(MODULE_ID, DEPENDENCY_MAP.paths)
630+
`);
631+
632+
const makeAsyncRequireTemplateWithName = template.expression(`
633+
require(ASYNC_REQUIRE_MODULE_PATH)(MODULE_ID, DEPENDENCY_MAP.paths, MODULE_NAME)
630634
`);
631635

632636
const makeAsyncPrefetchTemplate = template.expression(`
633-
require(ASYNC_REQUIRE_MODULE_PATH).prefetch(MODULE_ID, MODULE_NAME, DEPENDENCY_MAP.paths)
637+
require(ASYNC_REQUIRE_MODULE_PATH).prefetch(MODULE_ID, DEPENDENCY_MAP.paths)
638+
`);
639+
640+
const makeAsyncPrefetchTemplateWithName = template.expression(`
641+
require(ASYNC_REQUIRE_MODULE_PATH).prefetch(MODULE_ID, DEPENDENCY_MAP.paths, MODULE_NAME)
634642
`);
635643

636644
const makeResolveWeakTemplate = template.expression(`
@@ -661,33 +669,41 @@ const DefaultDependencyTransformer: DependencyTransformer = {
661669
dependency: InternalDependency,
662670
state: State,
663671
): void {
664-
path.replaceWith(
665-
makeAsyncRequireTemplate({
666-
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
667-
state.asyncRequireModulePathStringLiteral,
668-
),
669-
MODULE_ID: createModuleIDExpression(dependency, state),
670-
MODULE_NAME: createModuleNameLiteral(dependency),
671-
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
672-
}),
673-
);
672+
const makeNode = state.keepRequireNames
673+
? makeAsyncRequireTemplateWithName
674+
: makeAsyncRequireTemplate;
675+
const opts = {
676+
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
677+
state.asyncRequireModulePathStringLiteral,
678+
),
679+
MODULE_ID: createModuleIDExpression(dependency, state),
680+
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
681+
...(state.keepRequireNames
682+
? {MODULE_NAME: createModuleNameLiteral(dependency)}
683+
: null),
684+
};
685+
path.replaceWith(makeNode(opts));
674686
},
675687

676688
transformPrefetch(
677689
path: NodePath<>,
678690
dependency: InternalDependency,
679691
state: State,
680692
): void {
681-
path.replaceWith(
682-
makeAsyncPrefetchTemplate({
683-
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
684-
state.asyncRequireModulePathStringLiteral,
685-
),
686-
MODULE_ID: createModuleIDExpression(dependency, state),
687-
MODULE_NAME: createModuleNameLiteral(dependency),
688-
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
689-
}),
690-
);
693+
const makeNode = state.keepRequireNames
694+
? makeAsyncPrefetchTemplateWithName
695+
: makeAsyncPrefetchTemplate;
696+
const opts = {
697+
ASYNC_REQUIRE_MODULE_PATH: nullthrows(
698+
state.asyncRequireModulePathStringLiteral,
699+
),
700+
MODULE_ID: createModuleIDExpression(dependency, state),
701+
DEPENDENCY_MAP: nullthrows(state.dependencyMapIdentifier),
702+
...(state.keepRequireNames
703+
? {MODULE_NAME: createModuleNameLiteral(dependency)}
704+
: null),
705+
};
706+
path.replaceWith(makeNode(opts));
691707
},
692708

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

0 commit comments

Comments
 (0)