Skip to content

Commit

Permalink
ModuleResolver: Narrow arguments, don't require entire ModuleMap
Browse files Browse the repository at this point in the history
Summary:
The intention here is to make clear what `ModuleResolver` actually needs out of the (Haste) `ModuleMap`, so that we can more easily reason about whether it's safe to pass a snapshot, live object etc in subsequent diffs.

Some "duplication" due to `node-haste.js`, which is only referenced internally by Metro-Buck. I plan to get rid of that and consolidate the implementations, but it's not on the critical path for symlink support right now.

Changelog: Internal

Reviewed By: huntie

Differential Revision: D40837004

fbshipit-source-id: ae305836bc5736359be1de13bc6a361ec4368cc3
  • Loading branch information
robhogan authored and facebook-github-bot committed Nov 8, 2022
1 parent d1f3629 commit 951c1d9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 33 deletions.
29 changes: 17 additions & 12 deletions packages/metro/src/ModuleGraph/node-haste/node-haste.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,27 +169,32 @@ exports.createResolveFn = function (options: ResolveOptions): ResolveFn {
const assetExtensions = new Set(assetExts.map(asset => '.' + asset));
const isAssetFile = (file: string) => assetExtensions.has(path.extname(file));

const moduleMap = new ModuleMap({
duplicates: new Map(),
map: createModuleMap({
files,
moduleCache,
sourceExts: new Set(sourceExts),
additionalExts: new Set(additionalExts),
platforms,
}),
mocks: new Map(),
rootDir: '',
});

const moduleResolver = new ModuleResolver({
dirExists: (filePath: string): boolean => hasteFS.dirExists(filePath),
disableHierarchicalLookup: options.disableHierarchicalLookup,
doesFileExist: (filePath: string): boolean => hasteFS.exists(filePath),
emptyModulePath: options.emptyModulePath,
extraNodeModules,
getHasteModulePath: (name, platform) =>
moduleMap.getModule(name, platform, true),
getHastePackagePath: (name, platform) =>
moduleMap.getPackage(name, platform, true),
isAssetFile,
mainFields: options.mainFields,
moduleCache,
moduleMap: new ModuleMap({
duplicates: new Map(),
map: createModuleMap({
files,
moduleCache,
sourceExts: new Set(sourceExts),
additionalExts: new Set(additionalExts),
platforms,
}),
mocks: new Map(),
rootDir: '',
}),
nodeModulesPaths: options.nodeModulesPaths,
preferNativePlatform: true,
projectRoot: '',
Expand Down
5 changes: 4 additions & 1 deletion packages/metro/src/node-haste/DependencyGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,13 @@ class DependencyGraph extends EventEmitter {
doesFileExist: this._doesFileExist,
emptyModulePath: this._config.resolver.emptyModulePath,
extraNodeModules: this._config.resolver.extraNodeModules,
getHasteModulePath: (name, platform) =>
this._moduleMap.getModule(name, platform, true),
getHastePackagePath: (name, platform) =>
this._moduleMap.getPackage(name, platform, true),
isAssetFile: file => this._assetExtensions.has(path.extname(file)),
mainFields: this._config.resolver.resolverMainFields,
moduleCache: this._moduleCache,
moduleMap: this._moduleMap,
nodeModulesPaths: this._config.resolver.nodeModulesPaths,
preferNativePlatform: true,
projectRoot: this._config.projectRoot,
Expand Down
40 changes: 20 additions & 20 deletions packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

'use strict';

import type {ModuleMap} from 'metro-file-map';
import type {
CustomResolver,
DoesFileExist,
Expand Down Expand Up @@ -55,23 +54,24 @@ export type ModuleishCache<TModule, TPackage> = interface {
getPackageOf(modulePath: string): ?TPackage,
};

type Options<TModule, TPackage> = {
+dirExists: DirExistsFn,
+disableHierarchicalLookup: boolean,
+doesFileExist: DoesFileExist,
+emptyModulePath: string,
+extraNodeModules: ?Object,
+isAssetFile: IsAssetFile,
+mainFields: $ReadOnlyArray<string>,
+moduleCache: ModuleishCache<TModule, TPackage>,
+moduleMap: ModuleMap,
+nodeModulesPaths: $ReadOnlyArray<string>,
+preferNativePlatform: boolean,
+projectRoot: string,
+resolveAsset: ResolveAsset,
+resolveRequest: ?CustomResolver,
+sourceExts: $ReadOnlyArray<string>,
};
type Options<TModule, TPackage> = $ReadOnly<{
dirExists: DirExistsFn,
disableHierarchicalLookup: boolean,
doesFileExist: DoesFileExist,
emptyModulePath: string,
extraNodeModules: ?Object,
getHasteModulePath: (name: string, platform: ?string) => ?string,
getHastePackagePath: (name: string, platform: ?string) => ?string,
isAssetFile: IsAssetFile,
mainFields: $ReadOnlyArray<string>,
moduleCache: ModuleishCache<TModule, TPackage>,
nodeModulesPaths: $ReadOnlyArray<string>,
preferNativePlatform: boolean,
projectRoot: string,
resolveAsset: ResolveAsset,
resolveRequest: ?CustomResolver,
sourceExts: $ReadOnlyArray<string>,
}>;

class ModuleResolver<TModule: Moduleish, TPackage: Packageish> {
_options: Options<TModule, TPackage>;
Expand Down Expand Up @@ -183,9 +183,9 @@ class ModuleResolver<TModule: Moduleish, TPackage: Packageish> {
allowHaste,
platform,
resolveHasteModule: (name: string) =>
this._options.moduleMap.getModule(name, platform, true),
this._options.getHasteModulePath(name, platform),
resolveHastePackage: (name: string) =>
this._options.moduleMap.getPackage(name, platform, true),
this._options.getHastePackagePath(name, platform),
getPackageMainPath: this._getPackageMainPath,
},
moduleName,
Expand Down

0 comments on commit 951c1d9

Please sign in to comment.