From 439f0d0db02699171b2937b158c5aae2677e1c7f Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Mon, 20 Mar 2023 05:47:33 -0700 Subject: [PATCH] Remove unused collectDependencies extension points Summary: Changelog: Internal Removes the `unstable_collectDependenciesPath` top-level config option, as well as the `dependencyRegistry` parameter to `collectDependencies`. Reviewed By: huntie Differential Revision: D43477213 fbshipit-source-id: 5f98f25a6fab0632721aa9ec699352685db52182 --- .../__snapshots__/loadConfig-test.js.snap | 4 -- packages/metro-config/src/defaults/index.js | 2 - .../src/__tests__/index-test.js | 60 ------------------- packages/metro-transform-worker/src/index.js | 13 +--- .../__tests__/collectDependencies-test.js | 59 ------------------ .../ModuleGraph/worker/collectDependencies.js | 17 +----- 6 files changed, 5 insertions(+), 150 deletions(-) diff --git a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap index dfcf5b91cf..1146f694d2 100644 --- a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap +++ b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap @@ -146,7 +146,6 @@ Object { "default": Object {}, }, "unstable_allowRequireContext": false, - "unstable_collectDependenciesPath": "metro/src/ModuleGraph/worker/collectDependencies.js", "unstable_compactOutput": false, "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, @@ -324,7 +323,6 @@ Object { "default": Object {}, }, "unstable_allowRequireContext": false, - "unstable_collectDependenciesPath": "metro/src/ModuleGraph/worker/collectDependencies.js", "unstable_compactOutput": false, "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, @@ -502,7 +500,6 @@ Object { "default": Object {}, }, "unstable_allowRequireContext": false, - "unstable_collectDependenciesPath": "metro/src/ModuleGraph/worker/collectDependencies.js", "unstable_compactOutput": false, "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, @@ -680,7 +677,6 @@ Object { "default": Object {}, }, "unstable_allowRequireContext": false, - "unstable_collectDependenciesPath": "metro/src/ModuleGraph/worker/collectDependencies.js", "unstable_compactOutput": false, "unstable_dependencyMapReservedName": null, "unstable_disableModuleWrapping": false, diff --git a/packages/metro-config/src/defaults/index.js b/packages/metro-config/src/defaults/index.js index 00f70c379b..9c279097f4 100644 --- a/packages/metro-config/src/defaults/index.js +++ b/packages/metro-config/src/defaults/index.js @@ -129,8 +129,6 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({ publicPath: '/assets', allowOptionalDependencies: false, unstable_allowRequireContext: false, - unstable_collectDependenciesPath: - 'metro/src/ModuleGraph/worker/collectDependencies.js', unstable_dependencyMapReservedName: null, unstable_disableModuleWrapping: false, unstable_disableNormalizePseudoGlobals: false, diff --git a/packages/metro-transform-worker/src/__tests__/index-test.js b/packages/metro-transform-worker/src/__tests__/index-test.js index df815af649..910b47119b 100644 --- a/packages/metro-transform-worker/src/__tests__/index-test.js +++ b/packages/metro-transform-worker/src/__tests__/index-test.js @@ -59,8 +59,6 @@ const baseConfig: JsTransformerConfig = { minifierPath: 'minifyModulePath', optimizationSizeLimit: 100000, publicPath: '/assets', - unstable_collectDependenciesPath: - 'metro/src/ModuleGraph/worker/collectDependencies', unstable_dependencyMapReservedName: null, unstable_compactOutput: false, unstable_disableModuleWrapping: false, @@ -471,64 +469,6 @@ it('transforms a script to JS source and bytecode', async () => { ).not.toThrow(); }); -it('allows replacing the collectDependencies implementation', async () => { - jest.mock( - 'metro-transform-worker/__virtual__/collectModifiedDependencies', - () => - jest.fn((ast, opts) => { - const metroCoreCollectDependencies = jest.requireActual< - (empty, empty) => {dependencies: {map: mixed => mixed}}, - >('metro/src/ModuleGraph/worker/collectDependencies'); - const collectedDeps = metroCoreCollectDependencies(ast, opts); - return { - ...collectedDeps, - // $FlowFixMe[missing-local-annot] - dependencies: collectedDeps.dependencies.map(dep => ({ - ...dep, - name: 'modified_' + dep.name, - })), - }; - }), - {virtual: true}, - ); - - const config = { - ...baseConfig, - unstable_collectDependenciesPath: - 'metro-transform-worker/__virtual__/collectModifiedDependencies', - }; - const options = { - dev: true, - type: 'module', - }; - const result = await Transformer.transform( - config, - '/root', - 'local/file.js', - Buffer.from('require("foo")', 'utf8'), - // $FlowFixMe[prop-missing] Added when annotating Transformer. - options, - ); - - // $FlowIgnore[cannot-resolve-module] this is a virtual module - const collectModifiedDependencies = require('metro-transform-worker/__virtual__/collectModifiedDependencies'); - expect(collectModifiedDependencies).toHaveBeenCalledWith( - expect.objectContaining({type: 'File'}), - { - allowOptionalDependencies: config.allowOptionalDependencies, - asyncRequireModulePath: config.asyncRequireModulePath, - dynamicRequires: 'reject', - inlineableCalls: ['_$$_IMPORT_DEFAULT', '_$$_IMPORT_ALL'], - keepRequireNames: options.dev, - unstable_allowRequireContext: false, - dependencyMapName: null, - }, - ); - expect(result.dependencies).toEqual([ - expect.objectContaining({name: 'modified_foo'}), - ]); -}); - it('uses a reserved dependency map name and prevents it from being minified', async () => { const result = await Transformer.transform( {...baseConfig, unstable_dependencyMapReservedName: 'THE_DEP_MAP'}, diff --git a/packages/metro-transform-worker/src/index.js b/packages/metro-transform-worker/src/index.js index d15bd23a1a..ca26a0abeb 100644 --- a/packages/metro-transform-worker/src/index.js +++ b/packages/metro-transform-worker/src/index.js @@ -33,7 +33,6 @@ import type { DependencyTransformer, DynamicRequiresBehavior, } from 'metro/src/ModuleGraph/worker/collectDependencies'; -import typeof CollectDependenciesFn from 'metro/src/ModuleGraph/worker/collectDependencies'; const getMinifier = require('./utils/getMinifier'); const {transformFromAstSync} = require('@babel/core'); @@ -52,6 +51,7 @@ const countLines = require('metro/src/lib/countLines'); const { InvalidRequireCallError: InternalInvalidRequireCallError, } = require('metro/src/ModuleGraph/worker/collectDependencies'); +const collectDependencies = require('metro/src/ModuleGraph/worker/collectDependencies'); const generateImportNames = require('metro/src/ModuleGraph/worker/generateImportNames'); const JsFileWrapping = require('metro/src/ModuleGraph/worker/JsFileWrapping'); const nullthrows = require('nullthrows'); @@ -94,7 +94,6 @@ export type JsTransformerConfig = $ReadOnly<{ optimizationSizeLimit: number, publicPath: string, allowOptionalDependencies: AllowOptionalDependencies, - unstable_collectDependenciesPath: string, unstable_dependencyMapReservedName: ?string, unstable_disableModuleWrapping: boolean, unstable_disableNormalizePseudoGlobals: boolean, @@ -395,8 +394,6 @@ async function transformJS( dependencyMapName: config.unstable_dependencyMapReservedName, unstable_allowRequireContext: config.unstable_allowRequireContext, }; - // $FlowFixMe[unsupported-syntax] dynamic require - const collectDependencies: CollectDependenciesFn = require(config.unstable_collectDependenciesPath); ({ast, dependencies, dependencyMapName} = collectDependencies(ast, opts)); } catch (error) { if (error instanceof InternalInvalidRequireCallError) { @@ -724,19 +721,13 @@ module.exports = { }, getCacheKey: (config: JsTransformerConfig): string => { - const { - babelTransformerPath, - minifierPath, - unstable_collectDependenciesPath, - ...remainingConfig - } = config; + const {babelTransformerPath, minifierPath, ...remainingConfig} = config; const filesKey = getCacheKey([ require.resolve(babelTransformerPath), require.resolve(minifierPath), require.resolve('./utils/getMinifier'), require.resolve('./utils/assetTransformer'), - require.resolve(unstable_collectDependenciesPath), require.resolve('metro/src/ModuleGraph/worker/generateImportNames'), require.resolve('metro/src/ModuleGraph/worker/JsFileWrapping'), ...metroTransformPlugins.getTransformPluginCacheKeyFiles(), diff --git a/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js b/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js index a3ae41d470..ee219136b3 100644 --- a/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js +++ b/packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js @@ -14,10 +14,7 @@ import type {Dependency} from '../collectDependencies'; import type { DependencyTransformer, - ImportQualifier, InternalDependency, - ModuleDependencyRegistry, - MutableInternalDependency, Options, State, } from '../collectDependencies'; @@ -1389,34 +1386,6 @@ it('uses the dependency transformer specified in the options to transform the de ); }); -it('uses the dependency registry specified in the options to register dependencies', () => { - const ast = astFromCode(` - const a = require('b/lib/a'); - import b from 'b/lib/b'; - export {Banana} from 'Banana'; - - import("some/async/module").then(foo => {}); - __prefetchImport("a/prefetch/module"); - `); - - const {dependencies} = collectDependencies(ast, { - ...opts, - dependencyTransformer: MockDependencyTransformer, - dependencyRegistry: new MockModuleDependencyRegistry(), - }); - - expect(dependencies).toEqual([ - {name: 'b/lib/a', data: objectContaining({asyncType: null})}, - {name: 'b/lib/b', data: objectContaining({asyncType: null})}, - {name: 'Banana', data: objectContaining({asyncType: null})}, - {name: 'some/async/module', data: objectContaining({asyncType: 'async'})}, - { - name: 'a/prefetch/module', - data: objectContaining({asyncType: 'prefetch'}), - }, - ]); -}); - it('collects require.resolveWeak calls', () => { const ast = astFromCode(` require.resolveWeak("some/async/module"); @@ -1481,34 +1450,6 @@ function astFromCode(code: string): BabelNodeFile { }); } -// Mock registry that collects *all* `registerDependency` calls. -// 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 implements ModuleDependencyRegistry { - _dependencies: Array = []; - - registerDependency(qualifier: ImportQualifier): InternalDependency { - const data: MutableInternalDependency = { - index: this._dependencies.length, - name: qualifier.name, - asyncType: qualifier.asyncType, - isOptional: qualifier.optional ?? false, - locs: [], - - // Index = easy key for every dependency since we don't collapse/reorder - key: String(this._dependencies.length), - }; - - this._dependencies.push(data); - return data; - } - - getDependencies(): Array { - return this._dependencies; - } -} - // Mock transformer for dependencies. Uses a "readable" format // require() -> require(id, module name) // import() -> require(async moudle name).async(id, module name) diff --git a/packages/metro/src/ModuleGraph/worker/collectDependencies.js b/packages/metro/src/ModuleGraph/worker/collectDependencies.js index 705d7597d6..37c5335be6 100644 --- a/packages/metro/src/ModuleGraph/worker/collectDependencies.js +++ b/packages/metro/src/ModuleGraph/worker/collectDependencies.js @@ -72,7 +72,7 @@ export type InternalDependency = $ReadOnly; export type State = { asyncRequireModulePathStringLiteral: ?StringLiteral, dependencyCalls: Set, - dependencyRegistry: ModuleDependencyRegistry, + dependencyRegistry: DependencyRegistry, dependencyTransformer: DependencyTransformer, dynamicRequires: DynamicRequiresBehavior, dependencyMapIdentifier: ?Identifier, @@ -89,7 +89,6 @@ export type Options = $ReadOnly<{ inlineableCalls: $ReadOnlyArray, keepRequireNames: boolean, allowOptionalDependencies: AllowOptionalDependencies, - dependencyRegistry?: ModuleDependencyRegistry, dependencyTransformer?: DependencyTransformer, /** Enable `require.context` statements which can be used to import multiple files in a directory. */ unstable_allowRequireContext: boolean, @@ -101,15 +100,6 @@ export type CollectedDependencies = $ReadOnly<{ dependencies: $ReadOnlyArray, }>; -// Registry for the dependency of a module. -// Defines when dependencies should be collapsed. -// E.g. should a module that's once required optionally and once not -// be treated as the same or different dependencies. -export interface ModuleDependencyRegistry { - registerDependency(qualifier: ImportQualifier): InternalDependency; - getDependencies(): Array; -} - export interface DependencyTransformer { transformSyncRequire( path: NodePath, @@ -149,8 +139,7 @@ function collectDependencies( const state: State = { asyncRequireModulePathStringLiteral: null, dependencyCalls: new Set(), - dependencyRegistry: - options.dependencyRegistry ?? new DefaultModuleDependencyRegistry(), + dependencyRegistry: new DependencyRegistry(), dependencyTransformer: options.dependencyTransformer ?? DefaultDependencyTransformer, dependencyMapIdentifier: null, @@ -774,7 +763,7 @@ function getKeyForDependency(qualifier: ImportQualifier): string { return key; } -class DefaultModuleDependencyRegistry implements ModuleDependencyRegistry { +class DependencyRegistry { _dependencies: Map = new Map(); registerDependency(qualifier: ImportQualifier): InternalDependency {