From 5eeb88f5a90c9f4755d09e3066d96c48e943f719 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Wed, 20 Dec 2017 01:48:17 -0800 Subject: [PATCH] metro: ModuleResolution: get rid of TModule for inner file resolution functions Summary: This is one of the first step towards a generic and composable resolution algorithm. We shall get rid of `TModule` as it's an abstraction that doesn't bring us any benefit in the resolution algo, and we actually want to get rid of `Module.js` eventually. This changeset gets rid of `TModule` up to the place where I already got rid of the exception-based control flow mechanism. Next steps are to push the boundaries higher up both for error handling, and for removing `TModule`. Reviewed By: mjesun Differential Revision: D6603157 fbshipit-source-id: 551e9ccd93628f45452148ed40a817bdde3740ea --- .../src/node-haste/AssetResolutionCache.js | 4 +- .../DependencyGraph/ModuleResolution.js | 81 +++++++++++++------ 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/packages/metro/src/node-haste/AssetResolutionCache.js b/packages/metro/src/node-haste/AssetResolutionCache.js index c8e6665b..de906e40 100644 --- a/packages/metro/src/node-haste/AssetResolutionCache.js +++ b/packages/metro/src/node-haste/AssetResolutionCache.js @@ -76,11 +76,11 @@ class AssetResolutionCache { dirPath: string, assetName: string, platform: string | null, - ): $ReadOnlyArray { + ): ?$ReadOnlyArray { const results = this._assetsByDirPath.get(dirPath); const assets = results.get(assetName); if (assets == null) { - return EMPTY_ARRAY; + return null; } return assets .filter(asset => asset.platform == null || asset.platform === platform) diff --git a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js index 463b0b24..e0bb96a7 100644 --- a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js +++ b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js @@ -70,7 +70,7 @@ type Options = {| dirPath: string, assetName: string, platform: string | null, - ) => $ReadOnlyArray, + ) => ?$ReadOnlyArray, +sourceExts: Array, |}; @@ -98,10 +98,15 @@ type DirCandidates = type FileAndDirCandidates = {|+dir: DirCandidates, +file: FileCandidates|}; -type Resolution = - | {|+type: 'resolved', +module: TModule|} +type Result = + | {|+type: 'resolved', +resolution: TResolution|} | {|+type: 'failed', +candidates: TCandidates|}; +type AssetFileResolution = $ReadOnlyArray; +type FileResolution = + | {|+type: 'sourceFile', +filePath: string|} + | {|+type: 'assetFiles', +filePaths: AssetFileResolution|}; + /** * It may not be a great pattern to leverage exception just for "trying" things * out, notably for performance. We should consider replacing these functions @@ -279,7 +284,7 @@ class ModuleResolver { // Eventually we should aggregate the candidates so that we can // report them with more accuracy in the error below. if (result.type === 'resolved') { - return result.module; + return this._getFileResolvedModule(result.resolution); } } @@ -320,7 +325,7 @@ class ModuleResolver { ): TModule { const result = this._loadAsFileOrDir(potentialModulePath, platform); if (result.type === 'resolved') { - return result.module; + return this._getFileResolvedModule(result.resolution); } // We ignore the `file` candidates as a temporary measure before this // function is gotten rid of, because it's historically been ignored anyway. @@ -341,6 +346,24 @@ class ModuleResolver { ); } + /** + * FIXME: get rid of this function and of the reliance on `TModule` + * altogether, return strongly typed resolutions at the top-level instead. + */ + _getFileResolvedModule(resolution: FileResolution): TModule { + switch (resolution.type) { + case 'sourceFile': + return this._options.moduleCache.getModule(resolution.filePath); + case 'assetFiles': + // FIXME: we should forward ALL the paths/metadata, + // not just an arbitrary item! + const arbitrary = getArrayLowestItem(resolution.filePaths); + invariant(arbitrary != null, 'invalid asset resolution'); + return this._options.moduleCache.getAssetModule(arbitrary); + } + throw new Error('switch is not exhaustive'); + } + /** * In the NodeJS-style module resolution scheme we want to check potential * paths both as directories and as files. For example, `foo/bar` may resolve @@ -350,7 +373,7 @@ class ModuleResolver { _loadAsFileOrDir( potentialModulePath: string, platform: string | null, - ): Resolution { + ): Result { const dirPath = path.dirname(potentialModulePath); const fileNameHint = path.basename(potentialModulePath); const fileResult = this._loadAsFile(dirPath, fileNameHint, platform); @@ -372,17 +395,17 @@ class ModuleResolver { dirPath: string, fileNameHint: string, platform: string | null, - ): Resolution { + ): Result { if (this.isAssetFile(fileNameHint)) { - return this._loadAsAssetFile(dirPath, fileNameHint, platform); + const result = this._loadAsAssetFile(dirPath, fileNameHint, platform); + return mapResult(result, filePaths => ({type: 'assetFiles', filePaths})); } const candidateExts = []; const filePathPrefix = path.join(dirPath, fileNameHint); const context = {filePathPrefix, candidateExts}; const filePath = this._tryToResolveSourceFile(context, platform); if (filePath != null) { - const module = this._options.moduleCache.getModule(filePath); - return resolvedAs(module); + return resolvedAs({type: 'sourceFile', filePath}); } return failedFor({type: 'sourceFile', candidateExts}); } @@ -391,13 +414,15 @@ class ModuleResolver { dirPath: string, fileNameHint: string, platform: string | null, - ): Resolution { + ): Result { const {resolveAsset} = this._options; const assetNames = resolveAsset(dirPath, fileNameHint, platform); - const assetName = getArrayLowestItem(assetNames); - if (assetName != null) { - const assetPath = path.join(dirPath, assetName); - return resolvedAs(this._options.moduleCache.getAssetModule(assetPath)); + if (assetNames != null) { + return resolvedAs( + assetNames.map(assetName => { + return path.join(dirPath, assetName); + }), + ); } return failedFor({type: 'asset', name: fileNameHint}); } @@ -513,7 +538,7 @@ class ModuleResolver { _loadAsDir( potentialDirPath: string, platform: string | null, - ): Resolution { + ): Result { const packageJsonPath = path.join(potentialDirPath, 'package.json'); if (this._options.doesFileExist(packageJsonPath)) { return this._loadAsPackage(packageJsonPath, platform); @@ -535,7 +560,7 @@ class ModuleResolver { _loadAsPackage( packageJsonPath: string, platform: string | null, - ): Resolution { + ): Result { const package_ = this._options.moduleCache.getPackage(packageJsonPath); const mainPrefixPath = package_.getMain(); const dirPath = path.dirname(mainPrefixPath); @@ -593,18 +618,28 @@ function getArrayLowestItem(a: $ReadOnlyArray): string | void { return lowest; } -function resolvedAs( - module: TModule, -): Resolution { - return {type: 'resolved', module}; +function resolvedAs( + resolution: TResolution, +): Result { + return {type: 'resolved', resolution}; } -function failedFor( +function failedFor( candidates: TCandidates, -): Resolution { +): Result { return {type: 'failed', candidates}; } +function mapResult( + result: Result, + mapper: TResolution => TNewResolution, +): Result { + if (result.type === 'failed') { + return result; + } + return {type: 'resolved', resolution: mapper(result.resolution)}; +} + class UnableToResolveError extends Error { /** * File path of the module that tried to require a module, ex. `/js/foo.js`.