From 3b3e0aaf725cfa6907bf2c8b5fbc0da352d29efe Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 4 Oct 2024 06:46:41 -0700 Subject: [PATCH] Make ResolutionContext.fileSystemLookup stable and required Summary: Promote `ResolutionContext.fileSystemLookup` to stable and (for providers of `ResolutionContext`) make it mandatory. `fileSystemLookup` has two main advantages over `doesFileExist`: - It allows nonempty directory existence checks, which is important for resolver performance. - It exposes `realPath`. Metro has no other API for using the in-memory file system to resolve a path to a real path. The latter is critical for correct implementation of custom resolvers (non-real paths can mean duplication in the module graph, and broken Fast Refresh, whereas use of `fs` obscures traversed symlinks, which interferes with incremental resolution). From now on, we will require custom resolvers to return absolute, *real*, paths, and suggest that they use this method to do it. `ResolutionContext.doesFileExist` is consequently deprecated as redundant and potentially misleading for implementers.. Changelog: ``` - **[Breaking]**: [Custom resolvers](https://metrobundler.dev/docs/configuration#resolverequest) must now return absolute, real paths for any successful resolution. - **[Feature]**: Expose [`ResolutionContext.fileSystemLookup`](https://metrobundler.dev/docs/resolution#resolution-context) for performing file and directory existence checks and resolving real paths. ``` Reviewed By: huntie Differential Revision: D62374241 fbshipit-source-id: 7760bd4278736e6db27f994a7598784b6578ab9c --- docs/Resolution.md | 14 +++++- .../src/PackageExportsResolve.js | 13 ++--- .../src/__tests__/package-exports-test.js | 1 - .../metro-resolver/src/__tests__/utils.js | 28 +++++------ packages/metro-resolver/src/resolve.js | 48 ++++++++----------- packages/metro-resolver/src/types.js | 14 +++++- packages/metro-resolver/types/types.d.ts | 14 +++++- .../metro/src/node-haste/DependencyGraph.js | 2 +- .../DependencyGraph/ModuleResolution.js | 6 +-- 9 files changed, 78 insertions(+), 62 deletions(-) diff --git a/docs/Resolution.md b/docs/Resolution.md index e24efe232..d08a477b9 100644 --- a/docs/Resolution.md +++ b/docs/Resolution.md @@ -168,11 +168,19 @@ The set of file extensions used to identify asset files. Defaults to [`resolver. `true` if the resolution is for a development bundle, or `false` otherwise. -#### `doesFileExist: string => boolean` +#### `doesFileExist: string => boolean`
Deprecated
Returns `true` if the file with the given path exists, or `false` otherwise. -By default, Metro implements this by consulting an in-memory map of the filesystem that has been prepared in advance. This approach avoids disk I/O during module resolution. +The default implementation wraps [`fileSystemLookup`](#filesystemlookup-string--exists-true-type-fd-realpath-string--exists-false) - prefer using that directly. + +#### `fileSystemLookup: string => {exists: true, type: 'f'|'d', realPath: string} | {exists: false}` + +*Added in v0.81.0* + +Return information about the given absolute or project-relative path, following symlinks. A file "exists" if and only if it is watched, and a directory must be non-empty. The returned `realPath` is real and absolute. + +By default, Metro implements this by consulting an in-memory map of the filesystem that has been prepared in advance. This approach avoids disk I/O during module resolution and performs realpath resolution at negligible additional cost. #### `nodeModulesPaths: $ReadOnlyArray` @@ -267,6 +275,8 @@ type Resolution = | {type: 'assetFiles', filePaths: $ReadOnlyArray}; ``` +Returned paths (`filePath` and `filePaths`) must be *absolute* and *real*, such as the `realPath` returned by [`fileSystemLookup`](#filesystemlookup-string--exists-true-type-fd-realpath-string--exists-false). + When calling the default resolver with a non-null `resolveRequest` function, it represents a custom resolver and will always be called, fully replacing the default resolution logic. Inside a custom resolver, `resolveRequest` is set to the default resolver function, for easy chaining and customization. diff --git a/packages/metro-resolver/src/PackageExportsResolve.js b/packages/metro-resolver/src/PackageExportsResolve.js index b30b85dca..10cd13991 100644 --- a/packages/metro-resolver/src/PackageExportsResolve.js +++ b/packages/metro-resolver/src/PackageExportsResolve.js @@ -110,18 +110,11 @@ export function resolvePackageTargetFromExports( } } - if (context.unstable_fileSystemLookup != null) { - const lookupResult = context.unstable_fileSystemLookup(filePath); - if (lookupResult.exists && lookupResult.type === 'f') { - return { - type: 'sourceFile', - filePath: lookupResult.realPath, - }; - } - } else if (context.doesFileExist(filePath)) { + const lookupResult = context.fileSystemLookup(filePath); + if (lookupResult.exists && lookupResult.type === 'f') { return { type: 'sourceFile', - filePath, + filePath: lookupResult.realPath, }; } diff --git a/packages/metro-resolver/src/__tests__/package-exports-test.js b/packages/metro-resolver/src/__tests__/package-exports-test.js index 28bd19652..eef7c8851 100644 --- a/packages/metro-resolver/src/__tests__/package-exports-test.js +++ b/packages/metro-resolver/src/__tests__/package-exports-test.js @@ -34,7 +34,6 @@ describe('with package exports resolution disabled', () => { }), originModulePath: '/root/src/main.js', unstable_enablePackageExports: false, - unstable_fileSystemLookup: null, }; expect(Resolver.resolve(context, 'test-pkg', null)).toEqual({ diff --git a/packages/metro-resolver/src/__tests__/utils.js b/packages/metro-resolver/src/__tests__/utils.js index 76f856275..d8393fadc 100644 --- a/packages/metro-resolver/src/__tests__/utils.js +++ b/packages/metro-resolver/src/__tests__/utils.js @@ -54,20 +54,7 @@ export function createResolutionContext( (typeof fileMap[filePath] === 'string' || typeof fileMap[filePath].realPath === 'string'), extraNodeModules: null, - mainFields: ['browser', 'main'], - nodeModulesPaths: [], - preferNativePlatform: false, - redirectModulePath: (filePath: string) => filePath, - resolveAsset: (filePath: string) => null, - resolveHasteModule: (name: string) => null, - resolveHastePackage: (name: string) => null, - sourceExts: ['js', 'jsx', 'json', 'ts', 'tsx'], - unstable_conditionNames: ['require'], - unstable_conditionsByPlatform: { - web: ['browser'], - }, - unstable_enablePackageExports: false, - unstable_fileSystemLookup: inputPath => { + fileSystemLookup: inputPath => { // Normalise and remove any trailing slash. const filePath = path.resolve(inputPath); const candidate = fileMap[filePath]; @@ -89,6 +76,19 @@ export function createResolutionContext( realPath: candidate.realPath, }; }, + mainFields: ['browser', 'main'], + nodeModulesPaths: [], + preferNativePlatform: false, + redirectModulePath: (filePath: string) => filePath, + resolveAsset: (filePath: string) => null, + resolveHasteModule: (name: string) => null, + resolveHastePackage: (name: string) => null, + sourceExts: ['js', 'jsx', 'json', 'ts', 'tsx'], + unstable_conditionNames: ['require'], + unstable_conditionsByPlatform: { + web: ['browser'], + }, + unstable_enablePackageExports: false, unstable_logWarning: () => {}, ...createPackageAccessors(fileMap), }; diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index 1799358c0..0604ccd10 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -135,22 +135,20 @@ function resolve( const allDirPaths = nodeModulesPaths .map(nodeModulePath => { let lookupResult = null; - if (context.unstable_fileSystemLookup) { - // Insight: The module can only exist if there is a `node_modules` at - // this path. Redirections cannot succeed, because we will never look - // beyond a node_modules path segment for finding the closest - // package.json. Moreover, if the specifier contains a '/' separator, - // the first part *must* be a real directory, because it is the - // shallowest path that can possibly contain a redirecting package.json. - const mustBeDirectory = - parsedSpecifier.posixSubpath !== '.' || - parsedSpecifier.packageName.length > parsedSpecifier.firstPart.length - ? nodeModulePath + path.sep + parsedSpecifier.firstPart - : nodeModulePath; - lookupResult = context.unstable_fileSystemLookup(mustBeDirectory); - if (!lookupResult.exists || lookupResult.type !== 'd') { - return null; - } + // Insight: The module can only exist if there is a `node_modules` at + // this path. Redirections cannot succeed, because we will never look + // beyond a node_modules path segment for finding the closest + // package.json. Moreover, if the specifier contains a '/' separator, + // the first part *must* be a real directory, because it is the + // shallowest path that can possibly contain a redirecting package.json. + const mustBeDirectory = + parsedSpecifier.posixSubpath !== '.' || + parsedSpecifier.packageName.length > parsedSpecifier.firstPart.length + ? nodeModulePath + path.sep + parsedSpecifier.firstPart + : nodeModulePath; + lookupResult = context.fileSystemLookup(mustBeDirectory); + if (!lookupResult.exists || lookupResult.type !== 'd') { + return null; } return path.join(nodeModulePath, realModuleName); }) @@ -398,12 +396,8 @@ function resolvePackageEntryPoint( packagePath: string, platform: string | null, ): Result { - const dirLookup = context.unstable_fileSystemLookup?.(packagePath); - - if ( - dirLookup != null && - (dirLookup.exists === false || dirLookup.type !== 'd') - ) { + const dirLookup = context.fileSystemLookup(packagePath); + if (dirLookup.exists == false || dirLookup.type !== 'd') { return failedFor({ type: 'sourceFile', filePathPrefix: packagePath, @@ -575,13 +569,9 @@ function resolveSourceFileForExt( if (redirectedPath === false) { return {type: 'empty'}; } - if (context.unstable_fileSystemLookup) { - const lookupResult = context.unstable_fileSystemLookup(redirectedPath); - if (lookupResult.exists && lookupResult.type === 'f') { - return lookupResult.realPath; - } - } else if (context.doesFileExist(redirectedPath)) { - return redirectedPath; + const lookupResult = context.fileSystemLookup(redirectedPath); + if (lookupResult.exists && lookupResult.type === 'f') { + return lookupResult.realPath; } context.candidateExts.push(extension); return null; diff --git a/packages/metro-resolver/src/types.js b/packages/metro-resolver/src/types.js index fa4545cd1..e092e255f 100644 --- a/packages/metro-resolver/src/types.js +++ b/packages/metro-resolver/src/types.js @@ -124,6 +124,12 @@ export type ResolutionContext = $ReadOnly<{ assetExts: $ReadOnlySet, customResolverOptions: CustomResolverOptions, disableHierarchicalLookup: boolean, + + /** + * Determine whether a regular file exists at the given path. + * + * @deprecated, prefer `fileSystemLookup` + */ doesFileExist: DoesFileExist, extraNodeModules: ?{[string]: string, ...}, @@ -151,6 +157,13 @@ export type ResolutionContext = $ReadOnly<{ */ dependency?: TransformResultDependency, + /** + * Synchonously returns information about a given absolute path, including + * whether it exists, whether it is a file or directory, and its absolute + * real path. + */ + fileSystemLookup: FileSystemLookup, + /** * The ordered list of fields to read in `package.json` to resolve a main * entry point based on the "browser" field spec. @@ -189,7 +202,6 @@ export type ResolutionContext = $ReadOnly<{ [platform: string]: $ReadOnlyArray, }>, unstable_enablePackageExports: boolean, - unstable_fileSystemLookup?: ?FileSystemLookup, unstable_logWarning: (message: string) => void, }>; diff --git a/packages/metro-resolver/types/types.d.ts b/packages/metro-resolver/types/types.d.ts index 112191ed0..6d22a538b 100644 --- a/packages/metro-resolver/types/types.d.ts +++ b/packages/metro-resolver/types/types.d.ts @@ -101,6 +101,12 @@ export interface ResolutionContext { readonly allowHaste: boolean; readonly customResolverOptions: CustomResolverOptions; readonly disableHierarchicalLookup: boolean; + + /** + * Determine whether a regular file exists at the given path. + * + * @deprecated, prefer `fileSystemLookup` + */ readonly doesFileExist: DoesFileExist; readonly extraNodeModules?: {[key: string]: string}; @@ -127,6 +133,13 @@ export interface ResolutionContext { */ readonly dependency?: TransformResultDependency; + /** + * Synchonously returns information about a given absolute path, including + * whether it exists, whether it is a file or directory, and its absolute + * real path. + */ + readonly fileSystemLookup: FileSystemLookup; + /** * The ordered list of fields to read in `package.json` to resolve a main * entry point based on the "browser" field spec. @@ -165,7 +178,6 @@ export interface ResolutionContext { [platform: string]: ReadonlyArray; }>; unstable_enablePackageExports: boolean; - unstable_fileSystemLookup?: FileSystemLookup | null; unstable_logWarning: (message: string) => void; } diff --git a/packages/metro/src/node-haste/DependencyGraph.js b/packages/metro/src/node-haste/DependencyGraph.js index dc12cfebf..1ff5a9fa6 100644 --- a/packages/metro/src/node-haste/DependencyGraph.js +++ b/packages/metro/src/node-haste/DependencyGraph.js @@ -189,6 +189,7 @@ class DependencyGraph extends EventEmitter { doesFileExist: this._doesFileExist, emptyModulePath: this._config.resolver.emptyModulePath, extraNodeModules: this._config.resolver.extraNodeModules, + fileSystemLookup, getHasteModulePath: (name, platform) => this._hasteMap.getModule(name, platform, true), getHastePackagePath: (name, platform) => @@ -219,7 +220,6 @@ class DependencyGraph extends EventEmitter { this._config.resolver.unstable_conditionsByPlatform, unstable_enablePackageExports: this._config.resolver.unstable_enablePackageExports, - unstable_fileSystemLookup: fileSystemLookup, }); } diff --git a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js index 8e4982cc7..b0d481a80 100644 --- a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js +++ b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js @@ -66,6 +66,7 @@ type Options = $ReadOnly<{ doesFileExist: DoesFileExist, emptyModulePath: string, extraNodeModules: ?Object, + fileSystemLookup: FileSystemLookup, getHasteModulePath: (name: string, platform: ?string) => ?string, getHastePackagePath: (name: string, platform: ?string) => ?string, mainFields: $ReadOnlyArray, @@ -82,7 +83,6 @@ type Options = $ReadOnly<{ [platform: string]: $ReadOnlyArray, }>, unstable_enablePackageExports: boolean, - unstable_fileSystemLookup: ?FileSystemLookup, }>; class ModuleResolver { @@ -142,6 +142,7 @@ class ModuleResolver { disableHierarchicalLookup, doesFileExist, extraNodeModules, + fileSystemLookup, mainFields, nodeModulesPaths, preferNativePlatform, @@ -151,7 +152,6 @@ class ModuleResolver { unstable_conditionNames, unstable_conditionsByPlatform, unstable_enablePackageExports, - unstable_fileSystemLookup, } = this._options; try { @@ -164,6 +164,7 @@ class ModuleResolver { disableHierarchicalLookup, doesFileExist, extraNodeModules, + fileSystemLookup, mainFields, nodeModulesPaths, preferNativePlatform, @@ -173,7 +174,6 @@ class ModuleResolver { unstable_conditionNames, unstable_conditionsByPlatform, unstable_enablePackageExports, - unstable_fileSystemLookup, unstable_logWarning: this._logWarning, customResolverOptions: resolverOptions.customResolverOptions ?? {}, originModulePath: fromModule.path,