Skip to content

Commit

Permalink
Make ResolutionContext.fileSystemLookup stable and required
Browse files Browse the repository at this point in the history
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
  • Loading branch information
robhogan authored and facebook-github-bot committed Oct 4, 2024
1 parent fd7c3b1 commit 3b3e0aa
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 62 deletions.
14 changes: 12 additions & 2 deletions docs/Resolution.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` <div class="label deprecated">Deprecated</div>

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<string>`

Expand Down Expand Up @@ -267,6 +275,8 @@ type Resolution =
| {type: 'assetFiles', filePaths: $ReadOnlyArray<string>};
```

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.
Expand Down
13 changes: 3 additions & 10 deletions packages/metro-resolver/src/PackageExportsResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
28 changes: 14 additions & 14 deletions packages/metro-resolver/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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),
};
Expand Down
48 changes: 19 additions & 29 deletions packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
Expand Down Expand Up @@ -398,12 +396,8 @@ function resolvePackageEntryPoint(
packagePath: string,
platform: string | null,
): Result<Resolution, FileCandidates> {
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,
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 13 additions & 1 deletion packages/metro-resolver/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ export type ResolutionContext = $ReadOnly<{
assetExts: $ReadOnlySet<string>,
customResolverOptions: CustomResolverOptions,
disableHierarchicalLookup: boolean,

/**
* Determine whether a regular file exists at the given path.
*
* @deprecated, prefer `fileSystemLookup`
*/
doesFileExist: DoesFileExist,
extraNodeModules: ?{[string]: string, ...},

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -189,7 +202,6 @@ export type ResolutionContext = $ReadOnly<{
[platform: string]: $ReadOnlyArray<string>,
}>,
unstable_enablePackageExports: boolean,
unstable_fileSystemLookup?: ?FileSystemLookup,
unstable_logWarning: (message: string) => void,
}>;

Expand Down
14 changes: 13 additions & 1 deletion packages/metro-resolver/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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.
Expand Down Expand Up @@ -165,7 +178,6 @@ export interface ResolutionContext {
[platform: string]: ReadonlyArray<string>;
}>;
unstable_enablePackageExports: boolean;
unstable_fileSystemLookup?: FileSystemLookup | null;
unstable_logWarning: (message: string) => void;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/metro/src/node-haste/DependencyGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -219,7 +220,6 @@ class DependencyGraph extends EventEmitter {
this._config.resolver.unstable_conditionsByPlatform,
unstable_enablePackageExports:
this._config.resolver.unstable_enablePackageExports,
unstable_fileSystemLookup: fileSystemLookup,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type Options<TPackage> = $ReadOnly<{
doesFileExist: DoesFileExist,
emptyModulePath: string,
extraNodeModules: ?Object,
fileSystemLookup: FileSystemLookup,
getHasteModulePath: (name: string, platform: ?string) => ?string,
getHastePackagePath: (name: string, platform: ?string) => ?string,
mainFields: $ReadOnlyArray<string>,
Expand All @@ -82,7 +83,6 @@ type Options<TPackage> = $ReadOnly<{
[platform: string]: $ReadOnlyArray<string>,
}>,
unstable_enablePackageExports: boolean,
unstable_fileSystemLookup: ?FileSystemLookup,
}>;

class ModuleResolver<TPackage: Packageish> {
Expand Down Expand Up @@ -142,6 +142,7 @@ class ModuleResolver<TPackage: Packageish> {
disableHierarchicalLookup,
doesFileExist,
extraNodeModules,
fileSystemLookup,
mainFields,
nodeModulesPaths,
preferNativePlatform,
Expand All @@ -151,7 +152,6 @@ class ModuleResolver<TPackage: Packageish> {
unstable_conditionNames,
unstable_conditionsByPlatform,
unstable_enablePackageExports,
unstable_fileSystemLookup,
} = this._options;

try {
Expand All @@ -164,6 +164,7 @@ class ModuleResolver<TPackage: Packageish> {
disableHierarchicalLookup,
doesFileExist,
extraNodeModules,
fileSystemLookup,
mainFields,
nodeModulesPaths,
preferNativePlatform,
Expand All @@ -173,7 +174,6 @@ class ModuleResolver<TPackage: Packageish> {
unstable_conditionNames,
unstable_conditionsByPlatform,
unstable_enablePackageExports,
unstable_fileSystemLookup,
unstable_logWarning: this._logWarning,
customResolverOptions: resolverOptions.customResolverOptions ?? {},
originModulePath: fromModule.path,
Expand Down

0 comments on commit 3b3e0aa

Please sign in to comment.