Skip to content

Commit

Permalink
Delete Meta-only __conditionallySplitJsResource support
Browse files Browse the repository at this point in the history 
Summary:
Changelog: Internal

Removes the Meta-only `__conditionallySplitJsResource` feature from `collectDependencies`, and simplifies a lot of internal types that previously had to be generic. This is one of the prerequisites for open sourcing a stable version of the `asyncRequire` API for dev-mode bundle splitting.

Reviewed By: robhogan

Differential Revision: D42621297

fbshipit-source-id: 8ab033fb6f8c23a470d1bf35b88ffbbf7091241e
  • Loading branch information
@motiz88 @facebook-github-bot
motiz88 authored and facebook-github-bot committed Feb 27, 2023
1 parent 354d6e4 commit f1d905b
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 185 deletions.
9 changes: 2 additions & 7 deletions packages/metro-transform-worker/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,6 @@ export type BytecodeOutput = $ReadOnly<{
type: BytecodeFileType,
}>;

type DependencySplitCondition = $PropertyType<
$PropertyType<TransformResultDependency, 'data'>,
'splitCondition',
>;

type TransformResponse = $ReadOnly<{
dependencies: $ReadOnlyArray<TransformResultDependency>,
output: $ReadOnlyArray<JsOutput | BytecodeOutput>,
Expand Down Expand Up @@ -264,7 +259,7 @@ const compileToBytecode = (
return HermesCompiler.compile(code, options);
};

const disabledDependencyTransformer: DependencyTransformer<mixed> = {
const disabledDependencyTransformer: DependencyTransformer = {
transformSyncRequire: () => void 0,
transformImportCall: () => void 0,
transformJSResource: () => void 0,
Expand Down Expand Up @@ -402,7 +397,7 @@ async function transformJS(
unstable_allowRequireContext: config.unstable_allowRequireContext,
};
// $FlowFixMe[unsupported-syntax] dynamic require
const collectDependencies: CollectDependenciesFn<DependencySplitCondition> = require(config.unstable_collectDependenciesPath);
const collectDependencies: CollectDependenciesFn = require(config.unstable_collectDependenciesPath);
({ast, dependencies, dependencyMapName} = collectDependencies(ast, opts));
} catch (error) {
if (error instanceof InternalInvalidRequireCallError) {
Expand Down
6 changes: 0 additions & 6 deletions packages/metro/src/DeltaBundler/types.flow.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ export type TransformResultDependency = {
* If not null, this dependency is due to a dynamic `import()` or `__prefetchImport()` call.
*/
+asyncType: AsyncDependencyType | null,
/**
* The condition for splitting on this dependency edge.
*/
+splitCondition?: {
+mobileConfigName: string,
},
/**
* The dependency is enclosed in a try/catch block.
*/
Expand Down
91 changes: 28 additions & 63 deletions packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const nullthrows = require('nullthrows');
const {any, objectContaining} = expect;

const {InvalidRequireCallError} = collectDependencies;
const opts: Options<mixed> = {
const opts: Options = {
asyncRequireModulePath: 'asyncRequire',
dynamicRequires: 'reject',
inlineableCalls: [],
Expand Down Expand Up @@ -874,18 +874,6 @@ it('collects __jsResource calls', () => {
);
});

it('collects conditionallySplitJSResource calls', () => {
const ast = astFromCode(`
__conditionallySplitJSResource("some/async/module", {mobileConfigName: 'aaa'});
__conditionallySplitJSResource("some/async/module", {mobileConfigName: 'bbb'});
`);
const {dependencies} = collectDependencies(ast, opts);
expect(dependencies).toEqual([
{name: 'some/async/module', data: objectContaining({asyncType: 'async'})},
{name: 'asyncRequire', data: objectContaining({asyncType: null})},
]);
});

describe('import() prefetching', () => {
it('collects prefetch calls', () => {
const ast = astFromCode(`
Expand Down Expand Up @@ -1022,7 +1010,7 @@ describe('Evaluating static arguments', () => {

it('throws at runtime when requiring non-strings with special option', () => {
const ast = astFromCode('require(1)');
const opts: Options<mixed> = {
const opts: Options = {
asyncRequireModulePath: 'asyncRequire',
dynamicRequires: 'throwAtRuntime',
inlineableCalls: [],
Expand Down Expand Up @@ -1129,8 +1117,8 @@ it('records locations of dependencies', () => {
import type {s} from 'setup/something';
import('some/async/module').then(foo => {});
__jsResource('some/async/module');
__conditionallySplitJSResource('some/async/module', {mobileConfigName: 'aaa'});
__conditionallySplitJSResource('some/async/module', {mobileConfigName: 'bbb'});
require('foo'); __prefetchImport('baz');
interopRequireDefault(require('quux')); // Simulated Babel output
`;
Expand Down Expand Up @@ -1160,18 +1148,10 @@ it('records locations of dependencies', () => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #3 (some/async/module)
> 5 | __jsResource('some/async/module');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #3 (some/async/module)
> 6 | __conditionallySplitJSResource('some/async/module', {mobileConfigName: 'aaa'});
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #3 (some/async/module)
> 7 | __conditionallySplitJSResource('some/async/module', {mobileConfigName: 'bbb'});
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #3 (some/async/module)
> 4 | import('some/async/module').then(foo => {});
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #4 (asyncRequire)
> 5 | __jsResource('some/async/module');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #4 (asyncRequire)
> 6 | __conditionallySplitJSResource('some/async/module', {mobileConfigName: 'aaa'});
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #4 (asyncRequire)
> 7 | __conditionallySplitJSResource('some/async/module', {mobileConfigName: 'bbb'});
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #4 (asyncRequire)
> 8 | require('foo'); __prefetchImport('baz');
| ^^^^^^^^^^^^^^^^^^^^^^^^ dep #4 (asyncRequire)
> 8 | require('foo'); __prefetchImport('baz');
Expand All @@ -1184,7 +1164,7 @@ it('records locations of dependencies', () => {
});

describe('optional dependencies', () => {
const opts: Options<> = {
const opts: Options = {
asyncRequireModulePath: 'asyncRequire',
dynamicRequires: 'reject',
inlineableCalls: [],
Expand All @@ -1194,7 +1174,7 @@ describe('optional dependencies', () => {
unstable_allowRequireContext: false,
};
const validateDependencies = (
dependencies: $ReadOnlyArray<Dependency<void>>,
dependencies: $ReadOnlyArray<Dependency>,
expectedCount: number,
) => {
let hasAsync = false;
Expand Down Expand Up @@ -1368,7 +1348,6 @@ it('uses the dependency transformer specified in the options to transform the de
import("some/async/module").then(foo => {});
__jsResource("some/async/module");
__conditionallySplitJSResource("some/async/module", {mobileConfigName: 'aaa'});
__prefetchImport("some/async/module");
`);

Expand All @@ -1386,7 +1365,6 @@ it('uses the dependency transformer specified in the options to transform the de
export { Banana } from 'Banana';
require("asyncRequire").async(_dependencyMap[3], "some/async/module").then(foo => {});
require("asyncRequire").jsresource(_dependencyMap[3], "some/async/module");
require("asyncRequire").jsresource(_dependencyMap[3], "some/async/module");
require("asyncRequire").prefetch(_dependencyMap[4], "some/async/module");
`),
);
Expand All @@ -1400,7 +1378,6 @@ it('uses the dependency registry specified in the options to register dependenci
import("some/async/module").then(foo => {});
__jsResource("a/jsresouce");
__conditionallySplitJSResource("a/conditional/jsresource", {mobileConfigName: 'aaa'});
__prefetchImport("a/prefetch/module");
`);

Expand All @@ -1416,10 +1393,6 @@ it('uses the dependency registry specified in the options to register dependenci
{name: 'Banana', data: objectContaining({asyncType: null})},
{name: 'some/async/module', data: objectContaining({asyncType: 'async'})},
{name: 'a/jsresouce', data: objectContaining({asyncType: 'async'})},
{
name: 'a/conditional/jsresource',
data: objectContaining({splitCondition: {mobileConfigName: 'aaa'}}),
},
{
name: 'a/prefetch/module',
data: objectContaining({asyncType: 'prefetch'}),
Expand All @@ -1443,7 +1416,7 @@ it('collects require.resolveWeak calls', () => {
});

function formatDependencyLocs(
dependencies: $ReadOnlyArray<Dependency<mixed>>,
dependencies: $ReadOnlyArray<Dependency>,
code: any,
) {
return (
Expand Down Expand Up @@ -1474,7 +1447,7 @@ function adjustLocForCodeFrame(loc: BabelSourceLocation) {
function formatLoc(
loc: BabelSourceLocation,
depIndex: number,
dep: Dependency<mixed>,
dep: Dependency,
code: any,
) {
return codeFrameColumns(code, adjustLocForCodeFrame(loc), {
Expand All @@ -1495,15 +1468,11 @@ function astFromCode(code: string): BabelNodeFile {
// Allows to verify that the `registerDependency` function was called for each
// extracted dependency. Collapsing dependencies is implemented by specific
// `collectDependencies` implementations and should be tested there.
class MockModuleDependencyRegistry<TSplitCondition>
implements ModuleDependencyRegistry<TSplitCondition>
{
_dependencies: Array<InternalDependency<TSplitCondition>> = [];

registerDependency(
qualifier: ImportQualifier,
): InternalDependency<TSplitCondition> {
const data: MutableInternalDependency<TSplitCondition> = {
class MockModuleDependencyRegistry implements ModuleDependencyRegistry {
_dependencies: Array<InternalDependency> = [];

registerDependency(qualifier: ImportQualifier): InternalDependency {
const data: MutableInternalDependency = {
index: this._dependencies.length,
name: qualifier.name,
asyncType: qualifier.asyncType,
Expand All @@ -1514,15 +1483,11 @@ class MockModuleDependencyRegistry<TSplitCondition>
key: String(this._dependencies.length),
};

if (qualifier.splitCondition) {
data.splitCondition = qualifier.splitCondition.evaluate().value;
}

this._dependencies.push(data);
return data;
}

getDependencies(): Array<InternalDependency<TSplitCondition>> {
getDependencies(): Array<InternalDependency> {
return this._dependencies;
}
}
Expand All @@ -1532,11 +1497,11 @@ class MockModuleDependencyRegistry<TSplitCondition>
// import() -> require(async moudle name).async(id, module name)
// jsresource -> require(async moudle name).jsresource(id, module name)
// prefetch -> require(async moudle name).prefetch(id, module name)
const MockDependencyTransformer: DependencyTransformer<mixed> = {
const MockDependencyTransformer: DependencyTransformer = {
transformSyncRequire(
path: NodePath<>,
dependency: InternalDependency<mixed>,
state: State<mixed>,
dependency: InternalDependency,
state: State,
): void {
path.replaceWith(
t.callExpression(t.identifier('require'), [
Expand All @@ -1548,38 +1513,38 @@ const MockDependencyTransformer: DependencyTransformer<mixed> = {

transformImportCall(
path: NodePath<>,
dependency: InternalDependency<mixed>,
state: State<mixed>,
dependency: InternalDependency,
state: State,
): void {
transformAsyncRequire(path, dependency, state, 'async');
},

transformJSResource(
path: NodePath<>,
dependency: InternalDependency<mixed>,
state: State<mixed>,
dependency: InternalDependency,
state: State,
): void {
transformAsyncRequire(path, dependency, state, 'jsresource');
},

transformPrefetch(
path: NodePath<>,
dependency: InternalDependency<mixed>,
state: State<mixed>,
dependency: InternalDependency,
state: State,
): void {
transformAsyncRequire(path, dependency, state, 'prefetch');
},

transformIllegalDynamicRequire(path: NodePath<>, state: State<mixed>): void {
transformIllegalDynamicRequire(path: NodePath<>, state: State): void {
path.replaceWith(
t.callExpression(t.identifier('requireIllegalDynamicRequire'), []),
);
},
};

function createModuleIDExpression(
dependency: InternalDependency<mixed>,
state: State<mixed>,
dependency: InternalDependency,
state: State,
) {
return t.memberExpression(
nullthrows(state.dependencyMapIdentifier),
Expand All @@ -1590,8 +1555,8 @@ function createModuleIDExpression(

function transformAsyncRequire(
path: NodePath<>,
dependency: InternalDependency<mixed>,
state: State<mixed>,
dependency: InternalDependency,
state: State,
methodName: string,
): void {
const moduleID = createModuleIDExpression(dependency, state);
Expand Down
Loading

0 comments on commit f1d905b

Please sign in to comment.