Skip to content

Commit

Permalink
Resolution: Replace context.unstable_getRealPath with context.unstabl…
Browse files Browse the repository at this point in the history
…e_fileSystemLookup

Summary:
Replace `unstable_getRealPath`, which was introduced to bring symlink support to the resolver, with the more powerful `unstable_fileSystemLookup`.

We're going to need API(s) that can
 - Check existence of a file or directory
 - Get the real path, to allow resolvers to return real paths.

This API aims to roll everything into one to reduce multiple lookups as much as possible. (I plan to deprecate `context.fileExists` for that reason, because currently using it in practice necessitates a second lookup to determine the real path for a correct resolution.)

This mirrors some previous underlying implementation changes in D52255863 and D52390401 and brings the context API more in line, without exposing more than necessary. As with D52255863, we use `{exists: false}` rather than a `null` return to allow for extended information in a non-breaking change in future - in particular whether the lookup goes outside a watched folder.

Changelog: Internal

Reviewed By: huntie

Differential Revision: D59003947

fbshipit-source-id: 02db458be7701fb8c5fd5feac8eff109ebc6bf1a
  • Loading branch information
robhogan authored and facebook-github-bot committed Aug 12, 2024
1 parent 349ac23 commit 9fc9bf0
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 30 deletions.
30 changes: 29 additions & 1 deletion packages/metro-file-map/types/flow-types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ export interface FileSystem {
getAllFiles(): Path[];
getDependencies(file: Path): string[] | null;
getModuleName(file: Path): string | null;
getRealPath(file: Path): string | null;
getSerializableSnapshot(): FileData;
getSha1(file: Path): string | null;

Expand Down Expand Up @@ -208,6 +207,12 @@ export interface FileSystem {
*/
linkStats(file: Path): FileStats | null;

/**
* Return information about the given path, whether a directory or file.
* Always follow symlinks, and return a real path if it exists.
*/
lookup(mixedPath: Path): LookupResult;

matchFiles(opts: {
/* Filter relative paths against a pattern. */
filter?: RegExp | null;
Expand All @@ -226,6 +231,29 @@ export interface FileSystem {

export type Glob = string;

export type LookupResult =
| {
// The node is missing from the FileSystem implementation (note this
// could indicate an unwatched path, or a directory containing no watched
// files).
exists: false;
// The real, normal, absolute paths of any symlinks traversed.
links: ReadonlySet<string>;
// The real, normal, absolute path of the first path segment
// encountered that does not exist, or cannot be navigated through.
missing: string;
}
| {
exists: true;
// The real, normal, absolute paths of any symlinks traversed.
links: ReadonlySet<string>;
// The real, normal, absolute path of the file or directory.
realPath: string;
// Currently lookup always follows symlinks, so can only return
// directories or regular files, but this may be extended.
type: 'd' | 'f';
};

export interface HasteMap {
getModule(
name: string,
Expand Down
8 changes: 4 additions & 4 deletions packages/metro-resolver/src/PackageExportsResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ export function resolvePackageTargetFromExports(
}
}

if (context.unstable_getRealPath != null) {
const maybeRealPath = context.unstable_getRealPath(filePath);
if (maybeRealPath != null) {
if (context.unstable_fileSystemLookup != null) {
const lookupResult = context.unstable_fileSystemLookup(filePath);
if (lookupResult.exists && lookupResult.type === 'f') {
return {
type: 'sourceFile',
filePath: maybeRealPath,
filePath: lookupResult.realPath,
};
}
} else if (context.doesFileExist(filePath)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('with package exports resolution disabled', () => {
}),
originModulePath: '/root/src/main.js',
unstable_enablePackageExports: false,
unstable_getRealPath: null,
unstable_fileSystemLookup: null,
};

expect(Resolver.resolve(context, 'test-pkg', null)).toEqual({
Expand Down
18 changes: 14 additions & 4 deletions packages/metro-resolver/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,20 @@ export function createResolutionContext(
web: ['browser'],
},
unstable_enablePackageExports: false,
unstable_getRealPath: filePath =>
typeof fileMap[filePath] === 'string'
? filePath
: fileMap[filePath]?.realPath,
unstable_fileSystemLookup: filePath => {
const candidate = fileMap[filePath];
if (typeof candidate === 'string') {
return {exists: true, type: 'f', realPath: filePath};
}
if (candidate == null || candidate.realPath == null) {
return {exists: false};
}
return {
exists: true,
type: 'f',
realPath: candidate.realPath,
};
},
unstable_logWarning: () => {},
...createPackageAccessors(fileMap),
};
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-resolver/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export type {
FileAndDirCandidates,
FileCandidates,
FileResolution,
GetRealPath,
FileSystemLookup,
ResolutionContext,
Resolution,
ResolveAsset,
Expand Down
8 changes: 4 additions & 4 deletions packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,10 +500,10 @@ function resolveSourceFileForExt(
if (redirectedPath === false) {
return {type: 'empty'};
}
if (context.unstable_getRealPath) {
const maybeRealPath = context.unstable_getRealPath(redirectedPath);
if (maybeRealPath != null) {
return maybeRealPath;
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;
Expand Down
12 changes: 10 additions & 2 deletions packages/metro-resolver/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,15 @@ export type PackageForModule = $ReadOnly<{
* Check existence of a single file.
*/
export type DoesFileExist = (filePath: string) => boolean;
export type GetRealPath = (path: string) => ?string;

/**
* Performs a lookup against an absolute or project-relative path to determine
* whether it exists as a file or directory. Follows any symlinks, and returns
* a real absolute path on existence.
*/
export type FileSystemLookup = (
absoluteOrProjectRelativePath: string,
) => {exists: false} | {exists: true, type: 'f' | 'd', realPath: string};

/**
* Given a directory path and the base asset name, return a list of all the
Expand Down Expand Up @@ -181,7 +189,7 @@ export type ResolutionContext = $ReadOnly<{
[platform: string]: $ReadOnlyArray<string>,
}>,
unstable_enablePackageExports: boolean,
unstable_getRealPath?: ?GetRealPath,
unstable_fileSystemLookup?: ?FileSystemLookup,
unstable_logWarning: (message: string) => void,
}>;

Expand Down
11 changes: 9 additions & 2 deletions packages/metro-resolver/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,15 @@ export interface PackageForModule extends PackageInfo {
* Check existence of a single file.
*/
export type DoesFileExist = (filePath: string) => boolean;
export type GetRealPath = (path: string) => string | null;
export type IsAssetFile = (fileName: string) => boolean;
/**
* Performs a lookup against an absolute or project-relative path to determine
* whether it exists as a file or directory. Follows any symlinks, and returns
* a real absolute path on existence.
*/
export type FileSystemLookup = (
absoluteOrProjectRelativePath: string,
) => {exists: false} | {exists: true; type: 'f' | 'd'; realPath: string};

/**
* Given a directory path and the base asset name, return a list of all the
Expand Down Expand Up @@ -158,7 +165,7 @@ export interface ResolutionContext {
[platform: string]: ReadonlyArray<string>;
}>;
unstable_enablePackageExports: boolean;
unstable_getRealPath?: GetRealPath | null;
unstable_fileSystemLookup?: FileSystemLookup | null;
unstable_logWarning: (message: string) => void;
}

Expand Down
21 changes: 14 additions & 7 deletions packages/metro/src/node-haste/DependencyGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,16 @@ class DependencyGraph extends EventEmitter {
}

_createModuleResolver() {
const getRealPathIfFile = (path: string) => {
const fileSystemLookup = (path: string) => {
const result = this._fileSystem.lookup(path);
return result.exists && result.type === 'f' ? result.realPath : null;
if (result.exists) {
return {
exists: true,
realPath: result.realPath,
type: result.type,
};
}
return {exists: false};
};

this._moduleResolver = new ModuleResolver({
Expand Down Expand Up @@ -190,14 +197,14 @@ class DependencyGraph extends EventEmitter {
reporter: this._config.reporter,
resolveAsset: (dirPath: string, assetName: string, extension: string) => {
const basePath = dirPath + path.sep + assetName;
let assets = [
const assets = [
basePath + extension,
...this._config.resolver.assetResolutions.map(
resolution => basePath + '@' + resolution + 'x' + extension,
),
];

assets = assets.map(getRealPathIfFile).filter(Boolean);
]
.map(assetPath => fileSystemLookup(assetPath).realPath)
.filter(Boolean);

return assets.length ? assets : null;
},
Expand All @@ -208,7 +215,7 @@ class DependencyGraph extends EventEmitter {
this._config.resolver.unstable_conditionsByPlatform,
unstable_enablePackageExports:
this._config.resolver.unstable_enablePackageExports,
unstable_getRealPath: getRealPathIfFile,
unstable_fileSystemLookup: fileSystemLookup,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import type {
CustomResolver,
DoesFileExist,
FileCandidates,
GetRealPath,
FileSystemLookup,
Resolution,
ResolveAsset,
} from 'metro-resolver';
Expand Down Expand Up @@ -82,7 +82,7 @@ type Options<TPackage> = $ReadOnly<{
[platform: string]: $ReadOnlyArray<string>,
}>,
unstable_enablePackageExports: boolean,
unstable_getRealPath: ?GetRealPath,
unstable_fileSystemLookup: ?FileSystemLookup,
}>;

class ModuleResolver<TPackage: Packageish> {
Expand Down Expand Up @@ -151,7 +151,7 @@ class ModuleResolver<TPackage: Packageish> {
unstable_conditionNames,
unstable_conditionsByPlatform,
unstable_enablePackageExports,
unstable_getRealPath,
unstable_fileSystemLookup,
} = this._options;

try {
Expand All @@ -173,7 +173,7 @@ class ModuleResolver<TPackage: Packageish> {
unstable_conditionNames,
unstable_conditionsByPlatform,
unstable_enablePackageExports,
unstable_getRealPath,
unstable_fileSystemLookup,
unstable_logWarning: this._logWarning,
customResolverOptions: resolverOptions.customResolverOptions ?? {},
originModulePath: fromModule.path,
Expand Down

0 comments on commit 9fc9bf0

Please sign in to comment.