From 89b0cab7321e877f341e3aa4a70cb61b6d4e1691 Mon Sep 17 00:00:00 2001 From: aleclarson Date: Tue, 18 Sep 2018 14:37:44 -0400 Subject: [PATCH 1/5] feat: symlinks in node_modules - resolve symlinks inside "node_modules" directories - add `follow` function to `ResolutionContext` type - move "node_modules" logic into the `yieldPotentialPaths` function - feat: scope-only keys in `extraNodeModules` map - fix: always preserve scope when checking `extraNodeModules` map - fix: throw an error if `resolveRequest` fails to resolve a direct import - [BREAKING] refactor the `FailedToResolveNameError` class This commit adds support for symlinks in any `node_modules` directory. This includes scoped packages. Also, PNPM users rejoice! The `yieldPotentialPaths` function avoids extra work by lazily generating the potential module paths for indirect imports. Also, the resolver now skips over module paths that contain `/node_modules/node_modules/`. The `extraNodeModules` map now has support for keys like `@babel` to act as a fallback for all modules starting with `@babel/`. Previously, if the `resolveRequest` function failed to resolve a direct import, we would continue onto the "node_modules" search logic. The correct behavior is to throw an error. --- .../src/FailedToResolveNameError.js | 18 +--- packages/metro-resolver/src/resolve.js | 91 ++++++++++++------- packages/metro-resolver/src/types.js | 2 + .../src/ModuleGraph/node-haste/node-haste.js | 1 + .../metro/src/node-haste/DependencyGraph.js | 1 + .../DependencyGraph/ModuleResolution.js | 22 +---- packages/metro/src/node-haste/types.js | 1 + 7 files changed, 75 insertions(+), 61 deletions(-) diff --git a/packages/metro-resolver/src/FailedToResolveNameError.js b/packages/metro-resolver/src/FailedToResolveNameError.js index bf01dce3dc..67d24dd9cc 100644 --- a/packages/metro-resolver/src/FailedToResolveNameError.js +++ b/packages/metro-resolver/src/FailedToResolveNameError.js @@ -13,25 +13,17 @@ const path = require('path'); class FailedToResolveNameError extends Error { - dirPaths: $ReadOnlyArray; - extraPaths: $ReadOnlyArray; + modulePaths: $ReadOnlyArray; - constructor( - dirPaths: $ReadOnlyArray, - extraPaths: $ReadOnlyArray, - ) { - const displayDirPaths = dirPaths.concat(extraPaths); - const hint = displayDirPaths.length ? ' or in these directories:' : ''; + constructor(modulePaths: $ReadOnlyArray) { + const hint = modulePaths.length ? ' or at these locations:' : ''; super( `Module does not exist in the Haste module map${hint}\n` + - displayDirPaths - .map(dirPath => ` ${path.dirname(dirPath)}\n`) - .join(', ') + + modulePaths.map(modulePath => ` ${modulePath}\n`).join(', ') + '\n', ); - this.dirPaths = dirPaths; - this.extraPaths = extraPaths; + this.modulePaths = modulePaths; } } diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index 32bfeb4c7a..8023a9cc80 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -90,45 +90,74 @@ function resolve( return resolution; } } catch (error) {} + if (isDirectImport) { + throw new Error('Failed to resolve module: ' + realModuleName); + } } - const dirPaths = []; - for ( - let currDir = path.dirname(originModulePath); - currDir !== '.' && currDir !== path.parse(originModulePath).root; - currDir = path.dirname(currDir) - ) { - const searchPath = path.join(currDir, 'node_modules'); - dirPaths.push(path.join(searchPath, realModuleName)); - } + const modulePaths = []; + for (let modulePath of genModulePaths(context, realModuleName)) { + modulePath = context.redirectModulePath(modulePath); - const extraPaths = []; - const {extraNodeModules} = context; - if (extraNodeModules) { - let bits = path.normalize(moduleName).split(path.sep); - let packageName; - // Normalize packageName and bits for scoped modules - if (bits.length >= 2 && bits[0].startsWith('@')) { - packageName = bits.slice(0, 2).join('/'); - bits = bits.slice(1); - } else { - packageName = bits[0]; - } - if (extraNodeModules[packageName]) { - bits[0] = extraNodeModules[packageName]; - extraPaths.push(path.join.apply(path, bits)); + const result = resolveFileOrDir(context, modulePath, platform); + if (result.type === 'resolved') { + return result.resolution; } + + modulePaths.push(modulePath); } + throw new FailedToResolveNameError(modulePaths); +} - const allDirPaths = dirPaths.concat(extraPaths); - for (let i = 0; i < allDirPaths.length; ++i) { - const realModuleName = context.redirectModulePath(allDirPaths[i]); - const result = resolveFileOrDir(context, realModuleName, platform); - if (result.type === 'resolved') { - return result.resolution; +/** Generate the potential module paths */ +function* genModulePaths( + context: ResolutionContext, + toModuleName: string, +): Iterable { + const {extraNodeModules, follow, originModulePath} = context; + + /** + * Extract the scope and package name from the module name. + */ + let bits = path.normalize(toModuleName).split(path.sep); + let packageName, scopeName; + if (bits.length >= 2 && bits[0].startsWith('@')) { + packageName = bits.slice(0, 2).join('/'); + scopeName = bits[0]; + bits = bits.slice(2); + } else { + packageName = bits.shift(); + } + + /** + * Find the nearest "node_modules" directory that contains + * the imported package. + */ + const {root} = path.parse(originModulePath); + let parent = originModulePath; + do { + parent = path.dirname(parent); + if (path.basename(parent) !== 'node_modules') { + yield path.join( + follow(path.join(parent, 'node_modules', packageName)), + ...bits, + ); + } + } while (parent !== root); + + /** + * Check the user-provided `extraNodeModules` module map for a + * direct mapping to a directory that contains the imported package. + */ + if (extraNodeModules) { + parent = + extraNodeModules[packageName] || + (scopeName ? extraNodeModules[scopeName] : void 0); + + if (parent) { + yield path.join(follow(path.join(parent, packageName)), ...bits); } } - throw new FailedToResolveNameError(dirPaths, extraPaths); } /** diff --git a/packages/metro-resolver/src/types.js b/packages/metro-resolver/src/types.js index 56653a1793..cd5d866747 100644 --- a/packages/metro-resolver/src/types.js +++ b/packages/metro-resolver/src/types.js @@ -48,6 +48,7 @@ export type FileCandidates = */ export type DoesFileExist = (filePath: string) => boolean; export type IsAssetFile = (fileName: string) => boolean; +export type FollowFn = (filePath: string) => string; /** * Given a directory path and the base asset name, return a list of all the @@ -111,6 +112,7 @@ export type ResolutionContext = ModulePathContext & extraNodeModules: ?{[string]: string}, originModulePath: string, resolveRequest?: ?CustomResolver, + follow: FollowFn, }; export type CustomResolver = ( diff --git a/packages/metro/src/ModuleGraph/node-haste/node-haste.js b/packages/metro/src/ModuleGraph/node-haste/node-haste.js index 8253a48d9e..b860f06f81 100644 --- a/packages/metro/src/ModuleGraph/node-haste/node-haste.js +++ b/packages/metro/src/ModuleGraph/node-haste/node-haste.js @@ -143,6 +143,7 @@ exports.createResolveFn = function(options: ResolveOptions): ResolveFn { dirExists: (filePath: string): boolean => hasteFS.dirExists(filePath), doesFileExist: (filePath: string): boolean => hasteFS.exists(filePath), extraNodeModules, + follow: (filePath: string): string => hasteFS.follow(filePath), isAssetFile: (filePath: string): boolean => helpers.isAssetFile(filePath), mainFields: options.mainFields, moduleCache, diff --git a/packages/metro/src/node-haste/DependencyGraph.js b/packages/metro/src/node-haste/DependencyGraph.js index 15c2da8a55..3eb0515d3c 100644 --- a/packages/metro/src/node-haste/DependencyGraph.js +++ b/packages/metro/src/node-haste/DependencyGraph.js @@ -143,6 +143,7 @@ class DependencyGraph extends EventEmitter { _createModuleResolver() { this._moduleResolver = new ModuleResolver({ + follow: (filePath: string) => this._hasteFS.follow(filePath), dirExists: (filePath: string) => { try { return fs.lstatSync(filePath).isDirectory(); diff --git a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js index 1cbc5dded6..43e6c4765e 100644 --- a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js +++ b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js @@ -26,6 +26,7 @@ import type { ResolveAsset, } from 'metro-resolver'; +export type FollowFn = (filePath: string) => string; export type DirExistsFn = (filePath: string) => boolean; /** @@ -54,6 +55,7 @@ export type ModuleishCache = { }; type Options = {| + +follow: FollowFn, +dirExists: DirExistsFn, +doesFileExist: DoesFileExist, +extraNodeModules: ?Object, @@ -173,28 +175,14 @@ class ModuleResolver { ); } if (error instanceof Resolver.FailedToResolveNameError) { - const { - dirPaths, - extraPaths, - }: { - // $flowfixme these types are defined explicitly in FailedToResolveNameError but Flow refuses to recognize them here - dirPaths: $ReadOnlyArray, - extraPaths: $ReadOnlyArray, - } = error; - const displayDirPaths = dirPaths - .filter((dirPath: string) => this._options.dirExists(dirPath)) - .map(dirPath => path.relative(this._options.projectRoot, dirPath)) - .concat(extraPaths); - - const hint = displayDirPaths.length ? ' or in these directories:' : ''; + const {modulePaths} = error; + const hint = modulePaths.length ? ' or at these locations:' : ''; throw new UnableToResolveError( path.relative(this._options.projectRoot, fromModule.path), moduleName, [ `${moduleName} could not be found within the project${hint || '.'}`, - ...displayDirPaths.map( - (dirPath: string) => ` ${path.dirname(dirPath)}`, - ), + ...modulePaths.map(modulePath => ` ${modulePath}`), '\nIf you are sure the module exists, try these steps:', ' 1. Clear watchman watches: watchman watch-del-all', ' 2. Delete node_modules: rm -rf node_modules and run yarn install', diff --git a/packages/metro/src/node-haste/types.js b/packages/metro/src/node-haste/types.js index dd31464e7e..e3e0e04aa0 100644 --- a/packages/metro/src/node-haste/types.js +++ b/packages/metro/src/node-haste/types.js @@ -13,6 +13,7 @@ // TODO(cpojer): Create a jest-types repo. export type HasteFS = { exists(filePath: string): boolean, + follow(filePath: string): string, getAllFiles(): Array, getFileIterator(): Iterator, getModuleName(filePath: string): ?string, From b46da6545b4647f811bfcbed3118bc5a7f174cd4 Mon Sep 17 00:00:00 2001 From: aleclarson Date: Mon, 24 Dec 2018 22:34:36 -0500 Subject: [PATCH 2/5] fix: getClosestPackage method on DependencyGraph We cannot assume that `filePath` is _not_ a directory. --- packages/metro/src/node-haste/DependencyGraph.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/metro/src/node-haste/DependencyGraph.js b/packages/metro/src/node-haste/DependencyGraph.js index 3eb0515d3c..22425f813d 100644 --- a/packages/metro/src/node-haste/DependencyGraph.js +++ b/packages/metro/src/node-haste/DependencyGraph.js @@ -117,9 +117,9 @@ class DependencyGraph extends EventEmitter { } _getClosestPackage(filePath: string): ?string { - const parsedPath = path.parse(filePath); - const root = parsedPath.root; - let dir = parsedPath.dir; + const {root} = path.parse(filePath); + // The `filePath` may be a directory. + let dir = filePath; do { const candidate = path.join(dir, 'package.json'); if (this._hasteFS.exists(candidate)) { From a34702ba527da01494e16bb2af5b6defd97aebf8 Mon Sep 17 00:00:00 2001 From: aleclarson Date: Fri, 1 Feb 2019 17:07:58 -0500 Subject: [PATCH 3/5] feat: package deduplication Packages with the same name/version pair are deduplicated. --- packages/metro-resolver/src/resolve.js | 149 +++++------------- packages/metro-resolver/src/types.js | 8 +- .../metro/src/node-haste/DependencyGraph.js | 1 + .../DependencyGraph/ModuleResolution.js | 35 ++-- .../DependencyGraph/ResolutionRequest.js | 1 + packages/metro/src/node-haste/ModuleCache.js | 33 +++- packages/metro/src/node-haste/Package.js | 26 +-- 7 files changed, 105 insertions(+), 148 deletions(-) diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index 8023a9cc80..8dffedf599 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -14,7 +14,6 @@ const FailedToResolveNameError = require('./FailedToResolveNameError'); const FailedToResolvePathError = require('./FailedToResolvePathError'); const InvalidPackageError = require('./InvalidPackageError'); -const formatFileCandidates = require('./formatFileCandidates'); const isAbsolutePath = require('absolute-path'); const path = require('path'); @@ -26,7 +25,6 @@ import type { FileContext, FileOrDirContext, FileResolution, - HasteContext, ModulePathContext, ResolutionContext, Resolution, @@ -34,6 +32,12 @@ import type { Result, } from './types'; +type ModuleParts = { + +package: string, + +scope: string, + +file: string, +}; + function resolve( context: ResolutionContext, moduleName: string, @@ -55,20 +59,20 @@ function resolve( } const {originModulePath} = context; - + const normalizedName = normalizePath(realModuleName); const isDirectImport = - isRelativeImport(realModuleName) || isAbsolutePath(realModuleName); + isRelativeImport(normalizedName) || isAbsolutePath(normalizedName); // We disable the direct file loading to let the custom resolvers deal with it if (!resolveRequest && isDirectImport) { - // derive absolute path /.../node_modules/originModuleDir/realModuleName + // derive absolute path /.../node_modules/originModuleDir/normalizedName const fromModuleParentIdx = originModulePath.lastIndexOf('node_modules' + path.sep) + 13; const originModuleDir = originModulePath.slice( 0, originModulePath.indexOf(path.sep, fromModuleParentIdx), ); - const absPath = path.join(originModuleDir, realModuleName); + const absPath = path.join(originModuleDir, normalizedName); return resolveModulePath(context, absPath, platform); } @@ -76,72 +80,64 @@ function resolve( // to allow overriding imports. It could be part of the custom resolver, but // that's not the case right now. if (context.allowHaste && !isDirectImport) { - const normalizedName = normalizePath(realModuleName); - const result = resolveHasteName(context, normalizedName, platform); - if (result.type === 'resolved') { - return result.resolution; + const modulePath = context.resolveHasteModule(normalizedName); + if (modulePath != null) { + return {type: 'sourceFile', filePath: modulePath}; } } if (resolveRequest) { try { - const resolution = resolveRequest(context, realModuleName, platform); + const resolution = resolveRequest(context, normalizedName, platform); if (resolution) { return resolution; } } catch (error) {} if (isDirectImport) { - throw new Error('Failed to resolve module: ' + realModuleName); + throw new Error('Failed to resolve module: ' + normalizedName); } } + const parsedName = parseModuleName(normalizedName); const modulePaths = []; - for (let modulePath of genModulePaths(context, realModuleName)) { - modulePath = context.redirectModulePath(modulePath); - + for (let packagePath of genPackagePaths(context, parsedName)) { + packagePath = context.redirectPackage(packagePath); + const modulePath = context.redirectModulePath( + path.join(packagePath, parsedName.file), + ); const result = resolveFileOrDir(context, modulePath, platform); if (result.type === 'resolved') { return result.resolution; } - modulePaths.push(modulePath); } throw new FailedToResolveNameError(modulePaths); } -/** Generate the potential module paths */ -function* genModulePaths( +function parseModuleName(moduleName: string): ModuleParts { + const parts = moduleName.split(path.sep); + const scope = parts[0].startsWith('@') ? parts[0] : ''; + return { + scope, + package: parts.slice(0, scope ? 2 : 1).join(path.sep), + file: parts.slice(scope ? 2 : 1).join(path.sep), + }; +} + +function* genPackagePaths( context: ResolutionContext, - toModuleName: string, + parsedName: ModuleParts, ): Iterable { - const {extraNodeModules, follow, originModulePath} = context; - - /** - * Extract the scope and package name from the module name. - */ - let bits = path.normalize(toModuleName).split(path.sep); - let packageName, scopeName; - if (bits.length >= 2 && bits[0].startsWith('@')) { - packageName = bits.slice(0, 2).join('/'); - scopeName = bits[0]; - bits = bits.slice(2); - } else { - packageName = bits.shift(); - } - /** * Find the nearest "node_modules" directory that contains * the imported package. */ - const {root} = path.parse(originModulePath); - let parent = originModulePath; + const {root} = path.parse(context.originModulePath); + let parent = context.originModulePath; do { parent = path.dirname(parent); if (path.basename(parent) !== 'node_modules') { - yield path.join( - follow(path.join(parent, 'node_modules', packageName)), - ...bits, - ); + yield path.join(parent, 'node_modules', parsedName.package); } } while (parent !== root); @@ -149,13 +145,13 @@ function* genModulePaths( * Check the user-provided `extraNodeModules` module map for a * direct mapping to a directory that contains the imported package. */ - if (extraNodeModules) { - parent = - extraNodeModules[packageName] || - (scopeName ? extraNodeModules[scopeName] : void 0); - - if (parent) { - yield path.join(follow(path.join(parent, packageName)), ...bits); + if (context.extraNodeModules) { + const extras = context.extraNodeModules; + if ((parent = extras[parsedName.package])) { + yield path.join(parent, parsedName.package); + } + if (parsedName.scope && (parent = extras[parsedName.scope])) { + yield path.join(parent, parsedName.package); } } } @@ -186,65 +182,6 @@ function resolveModulePath( throw new FailedToResolvePathError(result.candidates); } -/** - * Resolve a module as a Haste module or package. For example we might try to - * resolve `Foo`, that is provided by file `/smth/Foo.js`. Or, in the case of - * a Haste package, it could be `/smth/Foo/index.js`. - */ -function resolveHasteName( - context: HasteContext, - moduleName: string, - platform: string | null, -): Result { - const modulePath = context.resolveHasteModule(moduleName); - if (modulePath != null) { - return resolvedAs({type: 'sourceFile', filePath: modulePath}); - } - let packageName = moduleName; - let packageJsonPath = context.resolveHastePackage(packageName); - while (packageJsonPath == null && packageName && packageName !== '.') { - packageName = path.dirname(packageName); - packageJsonPath = context.resolveHastePackage(packageName); - } - if (packageJsonPath == null) { - return failedFor(); - } - const packageDirPath = path.dirname(packageJsonPath); - const pathInModule = moduleName.substring(packageName.length + 1); - const potentialModulePath = path.join(packageDirPath, pathInModule); - const result = resolveFileOrDir(context, potentialModulePath, platform); - if (result.type === 'resolved') { - return result; - } - const {candidates} = result; - const opts = {moduleName, packageName, pathInModule, candidates}; - throw new MissingFileInHastePackageError(opts); -} - -class MissingFileInHastePackageError extends Error { - candidates: FileAndDirCandidates; - moduleName: string; - packageName: string; - pathInModule: string; - - constructor(opts: {| - +candidates: FileAndDirCandidates, - +moduleName: string, - +packageName: string, - +pathInModule: string, - |}) { - super( - `While resolving module \`${opts.moduleName}\`, ` + - `the Haste package \`${opts.packageName}\` was found. However the ` + - `module \`${opts.pathInModule}\` could not be found within ` + - 'the package. Indeed, none of these files exist:\n\n' + - ` * \`${formatFileCandidates(opts.candidates.file)}\`\n` + - ` * \`${formatFileCandidates(opts.candidates.dir)}\``, - ); - Object.assign(this, opts); - } -} - /** * 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 diff --git a/packages/metro-resolver/src/types.js b/packages/metro-resolver/src/types.js index cd5d866747..fcd7b62a36 100644 --- a/packages/metro-resolver/src/types.js +++ b/packages/metro-resolver/src/types.js @@ -90,12 +90,6 @@ export type HasteContext = FileOrDirContext & { * a Haste module of that name. Ex. for `Foo` it may return `/smth/Foo.js`. */ +resolveHasteModule: (name: string) => ?string, - /** - * Given a name, this should return the full path to the package manifest that - * provides a Haste package of that name. Ex. for `Foo` it may return - * `/smth/Foo/package.json`. - */ - +resolveHastePackage: (name: string) => ?string, }; export type ModulePathContext = FileOrDirContext & { @@ -110,7 +104,7 @@ export type ResolutionContext = ModulePathContext & HasteContext & { allowHaste: boolean, extraNodeModules: ?{[string]: string}, - originModulePath: string, + redirectPackage: (packagePath: string) => string, resolveRequest?: ?CustomResolver, follow: FollowFn, }; diff --git a/packages/metro/src/node-haste/DependencyGraph.js b/packages/metro/src/node-haste/DependencyGraph.js index 22425f813d..68058efb94 100644 --- a/packages/metro/src/node-haste/DependencyGraph.js +++ b/packages/metro/src/node-haste/DependencyGraph.js @@ -90,6 +90,7 @@ class DependencyGraph extends EventEmitter { resetCache: config.resetCache, rootDir: config.projectRoot, roots: config.watchFolders, + skipPackageJson: true, throwOnModuleCollision: true, useWatchman: config.resolver.useWatchman, watch: true, diff --git a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js index 43e6c4765e..81626ed9cb 100644 --- a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js +++ b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js @@ -92,7 +92,7 @@ class ModuleResolver { const fromPackagePath = './' + path.relative( - path.dirname(fromPackage.path), + fromPackage.root, path.resolve(path.dirname(fromModule.path), modulePath), ); @@ -109,19 +109,19 @@ class ModuleResolver { './' + path.relative( path.dirname(fromModule.path), - path.resolve(path.dirname(fromPackage.path), redirectedPath), + path.resolve(fromPackage.root, redirectedPath), ); } return redirectedPath; } } else { - const pck = path.isAbsolute(modulePath) + const pack = path.isAbsolute(modulePath) ? moduleCache.getModule(modulePath).getPackage() : fromModule.getPackage(); - if (pck) { - return pck.redirectRequire(modulePath, this._options.mainFields); + if (pack) { + return pack.redirectRequire(modulePath, this._options.mainFields); } } } catch (err) { @@ -147,11 +147,21 @@ class ModuleResolver { this._redirectRequire(fromModule, modulePath), allowHaste, platform, - resolveHasteModule: (name: string) => - this._options.moduleMap.getModule(name, platform, true), - resolveHastePackage: (name: string) => - this._options.moduleMap.getPackage(name, platform, true), - getPackageMainPath: this._getPackageMainPath, + resolveHasteModule(name) { + return this.moduleMap.getModule(name, platform, true); + }, + getPackageMainPath(packageJsonPath: string): string { + return this.moduleCache + .getPackage(packageJsonPath) + .getMain(this.mainFields); + }, + redirectPackage(packagePath: string): string { + packagePath = this.follow(packagePath); + const packageJsonPath = path.join(packagePath, 'package.json'); + return this.doesFileExist(packageJsonPath) + ? this.moduleCache.getPackage(packageJsonPath).root + : packagePath; + }, }, moduleName, platform, @@ -195,11 +205,6 @@ class ModuleResolver { } } - _getPackageMainPath = (packageJsonPath: string): string => { - const package_ = this._options.moduleCache.getPackage(packageJsonPath); - return package_.getMain(this._options.mainFields); - }; - /** * FIXME: get rid of this function and of the reliance on `TModule` * altogether, return strongly typed resolutions at the top-level instead. diff --git a/packages/metro/src/node-haste/DependencyGraph/ResolutionRequest.js b/packages/metro/src/node-haste/DependencyGraph/ResolutionRequest.js index 9de793fd2c..3a68119e7f 100644 --- a/packages/metro/src/node-haste/DependencyGraph/ResolutionRequest.js +++ b/packages/metro/src/node-haste/DependencyGraph/ResolutionRequest.js @@ -22,6 +22,7 @@ import type {ModuleResolver} from './ModuleResolution'; export type Packageish = { path: string, + root: string, redirectRequire( toModuleName: string, mainFields: $ReadOnlyArray, diff --git a/packages/metro/src/node-haste/ModuleCache.js b/packages/metro/src/node-haste/ModuleCache.js index bd17f12fc0..501a98b0f7 100644 --- a/packages/metro/src/node-haste/ModuleCache.js +++ b/packages/metro/src/node-haste/ModuleCache.js @@ -13,18 +13,22 @@ const Module = require('./Module'); const Package = require('./Package'); +import type {PackageContent} from './Package'; + type GetClosestPackageFn = (filePath: string) => ?string; class ModuleCache { _getClosestPackage: GetClosestPackageFn; _moduleCache: {[filePath: string]: Module, __proto__: null}; _packageCache: {[filePath: string]: Package, __proto__: null}; + _packagesById: {[id: string]: Package, __proto__: null}; _packageModuleMap: WeakMap; constructor(options: {getClosestPackage: GetClosestPackageFn}) { this._getClosestPackage = options.getClosestPackage; this._moduleCache = Object.create(null); this._packageCache = Object.create(null); + this._packagesById = Object.create(null); this._packageModuleMap = new WeakMap(); } @@ -36,12 +40,17 @@ class ModuleCache { } getPackage(filePath: string): Package { - if (!this._packageCache[filePath]) { - this._packageCache[filePath] = new Package({ + const cachedPackage = this._packageCache[filePath]; + if (!cachedPackage) { + const newPackage = new Package({ file: filePath, }); + const packageId = getPackageId(newPackage.read()); + return (this._packageCache[filePath] = + this._packagesById[packageId] || + (this._packagesById[packageId] = newPackage)); } - return this._packageCache[filePath]; + return cachedPackage; } getPackageForModule(module: Module): ?Package { @@ -59,8 +68,9 @@ class ModuleCache { return null; } - this._packageModuleMap.set(module, packagePath); - return this.getPackage(packagePath); + const pack = this.getPackage(packagePath); + this._packageModuleMap.set(module, pack.path); + return pack; } processFileChange(type: string, filePath: string) { @@ -68,11 +78,20 @@ class ModuleCache { this._moduleCache[filePath].invalidate(); delete this._moduleCache[filePath]; } - if (this._packageCache[filePath]) { - this._packageCache[filePath].invalidate(); + const pack = this._packageCache[filePath]; + if (pack) { + if (pack.content) { + const packageId = getPackageId(pack.content); + delete this._packagesById[packageId]; + } + pack.invalidate(); delete this._packageCache[filePath]; } } } +function getPackageId(content: PackageContent) { + return content.name + '@' + content.version; +} + module.exports = ModuleCache; diff --git a/packages/metro/src/node-haste/Package.js b/packages/metro/src/node-haste/Package.js index e1c2cfe7b3..ae2b6a0067 100644 --- a/packages/metro/src/node-haste/Package.js +++ b/packages/metro/src/node-haste/Package.js @@ -13,8 +13,9 @@ const fs = require('fs'); const path = require('path'); -type PackageContent = { +export type PackageContent = { name: string, + version: string, 'react-native': mixed, browser: mixed, main: ?string, @@ -22,14 +23,13 @@ type PackageContent = { class Package { path: string; - - _root: string; - _content: ?PackageContent; + root: string; + content: ?PackageContent; constructor({file}: {file: string}) { this.path = path.resolve(file); - this._root = path.dirname(this.path); - this._content = null; + this.root = path.dirname(this.path); + this.content = null; } /** @@ -76,11 +76,11 @@ class Package { } /* $FlowFixMe: `getReplacements` doesn't validate the return value. */ - return path.join(this._root, main); + return path.join(this.root, main); } invalidate() { - this._content = null; + this.content = null; } redirectRequire( @@ -101,7 +101,7 @@ class Package { } let relPath = - './' + path.relative(this._root, path.resolve(this._root, name)); + './' + path.relative(this.root, path.resolve(this.root, name)); if (path.sep !== '/') { relPath = relPath.replace(new RegExp('\\' + path.sep, 'g'), '/'); @@ -123,17 +123,17 @@ class Package { } if (redirect) { - return path.join(this._root, redirect); + return path.join(this.root, redirect); } return name; } read(): PackageContent { - if (this._content == null) { - this._content = JSON.parse(fs.readFileSync(this.path, 'utf8')); + if (this.content == null) { + this.content = JSON.parse(fs.readFileSync(this.path, 'utf8')); } - return this._content; + return this.content; } } From d946dee00976fb9cab566a0525bd188e1058c113 Mon Sep 17 00:00:00 2001 From: Alec Larson Date: Tue, 16 Apr 2019 18:00:46 -0400 Subject: [PATCH 4/5] fix: preserve leading "./" in `normalizePath` result --- packages/metro-resolver/src/resolve.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index 8dffedf599..c660835fbe 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -421,12 +421,16 @@ function isRelativeImport(filePath: string) { } function normalizePath(modulePath) { + const wasRelative = isRelativeImport(modulePath); if (path.sep === '/') { modulePath = path.normalize(modulePath); } else if (path.posix) { modulePath = path.posix.normalize(modulePath); } - + // Ensure `normalize` cannot strip leading "./" + if (wasRelative && modulePath[0] !== '.') { + modulePath = './' + modulePath; + } return modulePath.replace(/\/$/, ''); } From 9bd34fb36471e7cadf853ed3ebce4be517807eef Mon Sep 17 00:00:00 2001 From: Alec Larson Date: Sat, 16 Nov 2019 14:34:45 -0500 Subject: [PATCH 5/5] fix: skip redirectModulePath call for package imports An example "package import" is `require("foo")` Contrast that with a "file import" like `require("foo/bar")` --- packages/metro-resolver/src/resolve.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index c660835fbe..54c8d6c9bb 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -102,9 +102,10 @@ function resolve( const modulePaths = []; for (let packagePath of genPackagePaths(context, parsedName)) { packagePath = context.redirectPackage(packagePath); - const modulePath = context.redirectModulePath( - path.join(packagePath, parsedName.file), - ); + const modulePath = parsedName.file + ? context.redirectModulePath(path.join(packagePath, parsedName.file)) + : packagePath; + const result = resolveFileOrDir(context, modulePath, platform); if (result.type === 'resolved') { return result.resolution;