From dc14c62cb4eef881f35f9dbf90c0d3410fd6a164 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 9 Oct 2026 16:19:51 +0100 Subject: [PATCH 1/5] Resolver: Extract redirectPackageSubpath from redirectModulePath ## Summary `redirectModulePath` asks `context.getPackageForModule` for the package containing a module path, then applies that package's `mainFields` redirections (the "browser" spec) to the module's package-relative path. This splits the second half out as `redirectPackageSubpath`, for callers that already know the containing package. `redirectModulePath` delegates to it for relative and absolute paths and behaves exactly as before; bare specifiers are unchanged. https://github.com/react/metro/pull/1945 uses it to look up the package of a directory's files once and apply it to every extension candidate, instead of calling `redirectModulePath` per candidate. ## Test plan ``` yarn jest packages/metro-resolver packages/metro/src/DeltaBundler/__tests__/resolver-test.js yarn flow check ``` Changelog: Internal --- packages/metro-resolver/src/PackageResolve.js | 76 +++++++++++++------ 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/packages/metro-resolver/src/PackageResolve.js b/packages/metro-resolver/src/PackageResolve.js index 568d5f2aee..f327610a55 100644 --- a/packages/metro-resolver/src/PackageResolve.js +++ b/packages/metro-resolver/src/PackageResolve.js @@ -106,8 +106,6 @@ export function redirectModulePath( return modulePath; } - let redirectedPath; - if (modulePath.startsWith('.') || isModulePathAbsolute) { const packageRelativeModulePath = isModulePathAbsolute ? // If the module path is absolute, containingPackage is relative to it @@ -120,33 +118,23 @@ export function redirectModulePath( path.dirname(containingPackage.packageRelativePath), modulePath, ); - - redirectedPath = matchSubpathFromMainFields( - // Use prefixed POSIX path for lookup in package.json - './' + systemToPosixPath(packageRelativeModulePath), - containingPackage.packageJson, - mainFields, - ); - - if (typeof redirectedPath === 'string') { - // BRITTLE ASSUMPTION: This is always treated as a package-relative path - // and is converted back, even if the redirected path is a specifier - // referring to another package. - redirectedPath = path.isAbsolute(redirectedPath) - ? path.normalize(redirectedPath) - : path.join(containingPackage.rootPath, redirectedPath); - } - } else { - // Otherwise, `modulePath` may be an unprefixed relative path or a bare - // specifier (can also be an absolute specifier prefixed with a URL scheme). - // This is used only by the "browser" spec. - redirectedPath = matchSubpathFromMainFields( + return redirectPackageSubpath( + context, modulePath, - containingPackage.packageJson, - mainFields, + containingPackage, + packageRelativeModulePath, ); } + // Otherwise, `modulePath` may be an unprefixed relative path or a bare + // specifier (can also be an absolute specifier prefixed with a URL scheme). + // This is used only by the "browser" spec. + const redirectedPath = matchSubpathFromMainFields( + modulePath, + containingPackage.packageJson, + mainFields, + ); + if (redirectedPath != null) { return redirectedPath; } @@ -154,6 +142,44 @@ export function redirectModulePath( return modulePath; } +/** + * `redirectModulePath` for a module path whose containing package is already + * known. `packageRelativePath` is the path of the module relative to the + * package root, with system separators. Returns `false` if the module should + * be ignored, and `modulePath` if no `package.json` mapping is matched. + */ +export function redirectPackageSubpath( + context: Readonly<{ + mainFields: ResolutionContext['mainFields'], + ... + }>, + modulePath: string, + containingPackage: Readonly<{ + packageJson: PackageInfo['packageJson'], + rootPath: PackageInfo['rootPath'], + ... + }>, + packageRelativePath: string, +): string | false { + const redirectedPath = matchSubpathFromMainFields( + // Use prefixed POSIX path for lookup in package.json + './' + systemToPosixPath(packageRelativePath), + containingPackage.packageJson, + context.mainFields, + ); + + if (typeof redirectedPath === 'string') { + // BRITTLE ASSUMPTION: This is always treated as a package-relative path + // and is converted back, even if the redirected path is a specifier + // referring to another package. + return path.isAbsolute(redirectedPath) + ? path.normalize(redirectedPath) + : path.join(containingPackage.rootPath, redirectedPath); + } + + return redirectedPath === false ? false : modulePath; +} + /** * Get the mapped replacement for the given subpath defined by matching * `mainFields` entries in the passed `package.json` From 7e3174df9627ca65c06e74e719e641998c338d56 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 18 Sep 2026 16:36:14 +0100 Subject: [PATCH 2/5] Resolver: Resolve the package of a directory once for all extension candidates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on https://github.com/react/metro/pull/2044. ## Summary `resolveFile` tries each candidate extension in turn, and for each one `resolveSourceFileForExt` calls `redirectModulePath`, which asks `context.getPackageForModule` for the package scope of the candidate path. Every candidate in a directory belongs to the same package, so that's the same question asked once per extension - and because Metro's `PackageCache` memoises by module path, each distinct candidate string misses the first time it's seen, mostly for files that don't exist. This asks once, up front, for the package of the files in the directory, and shares it with every candidate via the source file context, applying each candidate's `mainFields` redirections with `redirectPackageSubpath` from the PR beneath. The package of the files in a directory isn't always the package of the directory itself - a `node_modules` directory belongs to whatever contains it while the files in it belong to none, and a symlink can change which directory along the way is named `node_modules`. So `getCandidatePackageScope` asks about a stand-in file in the directory, `.metro-package-scope`, which gets the same answer any real candidate there would. If that name happens to exist as a package root, the answer is about that package instead, which shows in its package-relative path, so it tries another name, and throws if `getPackageForModule` keeps answering about something else. `package-scope-test.js` pins the sequence of `getPackageForModule` calls the resolver makes for the common specifier shapes, so that the call count is a tested property rather than incidental. Two `resolver-test.js` cases pin the package scope of files reached through a symlink to or from a `node_modules` directory, where the directory and the files in it disagree. This also removes the `@deprecated` tag from `getPackageForModule` - the resolver depends on it, and its doc now says what it returns for symlinked and missing paths. ## Benchmark This cuts package scope lookups 3.7x, and takes 28% off first-pass resolution time, measured in isolation on our benchmark app (described in the test plan). 7 interleaved rounds. Resolver output is byte-identical. | metric | #1944 | this diff | |---|---|---| | `getPackageForModule` calls per pass | 64k | **17k (3.7x fewer)** | | first pass after startup (ms) | 119.6 | **85.7 (−28.3%)** | | warm pass (ms) | 55.2 | **53.5 (−3.1%)** | | after a source change (ms) | 63.0 | **60.9 (−3.4%)** | | after a `package.json` change (ms) | 68.4 | **64.0 (−6.4%)** | The first pass is where this lands, because that's where every distinct candidate path is a `PackageCache` miss and a walk up the tree. ## Test plan ``` yarn jest packages/metro-resolver packages/metro-file-map packages/metro/src/node-haste packages/metro/src/DeltaBundler/__tests__/resolver-test.js yarn flow check yarn verify-api-snapshots ``` **Benchmark app** - a synthetic mid-size RN 0.87 app: 23 common dependencies (Reanimated, React Navigation, TanStack Query, lodash, etc.) and ~1,000 generated first-party modules, 60% of them in a workspace package outside `projectRoot` with its own `node_modules`. The iOS dev bundle is 2.8k modules, from a file map of 34k files. Timings replay the resolutions a real build of it makes against a fresh `DependencyGraph` per process, in interleaved rounds. These timings isolate resolution - no transformation, serialisation or file reads are included. A real build of this app takes around 4s even with a warm transform cache, so resolution is about 2% of it. Changelog: Internal --- .../src/__tests__/package-scope-test.js | 226 ++++++++++++++++++ packages/metro-resolver/src/resolve.js | 75 +++++- packages/metro-resolver/src/types.js | 11 +- .../DeltaBundler/__tests__/resolver-test.js | 48 ++++ 4 files changed, 357 insertions(+), 3 deletions(-) create mode 100644 packages/metro-resolver/src/__tests__/package-scope-test.js diff --git a/packages/metro-resolver/src/__tests__/package-scope-test.js b/packages/metro-resolver/src/__tests__/package-scope-test.js new file mode 100644 index 0000000000..953afe8dc1 --- /dev/null +++ b/packages/metro-resolver/src/__tests__/package-scope-test.js @@ -0,0 +1,226 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + * @oncall react_native + */ + +import type {PackageForModule, ResolutionContext} from '../types'; + +import * as Resolver from '../index'; +import { + createPackageAccessors, + createResolutionContext, + posixToSystemPath as p, +} from './utils'; + +// The resolver looks up package scopes through `getPackageForModule`. These +// tests pin down which paths it asks about for each kind of specifier, so +// that changes to the number of lookups per resolution are deliberate. Metro +// memoizes the answer per path, so a repeated path is cheap and a new one is +// not: in particular, the files in a directory are looked up once, through a +// stand-in file name, for all of their extension candidates. +describe('package scope lookups', () => { + // The path the resolver asks about for the files in a directory + const filesIn = (dirPath: string) => p(dirPath + '/.metro-package-scope'); + + const fileMap = { + [p('/root/package.json')]: JSON.stringify({name: 'root'}), + [p('/root/src/main.js')]: '', + [p('/root/src/foo.js')]: '', + [p('/root/src/lib/index.js')]: '', + [p('/root/node_modules/pkg/package.json')]: JSON.stringify({ + name: 'pkg', + main: 'lib/index', + browser: {'./lib/redirected.js': './lib/other.js'}, + }), + [p('/root/node_modules/pkg/lib/index.js')]: '', + [p('/root/node_modules/pkg/lib/sub.js')]: '', + [p('/root/node_modules/pkg/lib/other.js')]: '', + [p('/root/node_modules/nopkg/index.js')]: '', + }; + + let getPackageForModule: JestMockFn<[string], ?PackageForModule>; + let context: ResolutionContext; + + function useFileMap(files: typeof fileMap) { + const accessors = createPackageAccessors(files); + getPackageForModule = jest.fn(accessors.getPackageForModule); + context = { + ...createResolutionContext(files), + ...accessors, + getPackageForModule, + originModulePath: p('/root/src/main.js'), + }; + } + + beforeEach(() => { + useFileMap(fileMap); + }); + + const lookedUpPaths = () => + getPackageForModule.mock.calls.map(([modulePath]) => modulePath); + + test('relative file, found after trying several extensions', () => { + expect(Resolver.resolve(context, './foo', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/src/foo.js'), + }); + expect(lookedUpPaths()).toEqual([p('/root/src/foo'), filesIn('/root/src')]); + }); + + test('relative file, when the stand-in file name is a package root', () => { + useFileMap({ + ...fileMap, + // A package whose "browser" field must not apply to the files beside it + [p('/root/src/.metro-package-scope/package.json')]: JSON.stringify({ + name: 'stand-in', + browser: {'./foo.js': false}, + }), + }); + expect(Resolver.resolve(context, './foo', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/src/foo.js'), + }); + // The stand-in is retried with another name + expect(lookedUpPaths()).toEqual([ + p('/root/src/foo'), + filesIn('/root/src'), + filesIn('/root/src') + '_', + ]); + }); + + test('throws if the scope is never that of the stand-in file', () => { + getPackageForModule = jest.fn(() => ({ + rootPath: p('/root'), + packageJson: {name: 'root'}, + packageRelativePath: '', + })); + context = {...context, getPackageForModule}; + expect(() => Resolver.resolve(context, './foo', 'ios')).toThrow( + `getPackageForModule(${JSON.stringify(filesIn('/root/src') + '__')}) ` + + 'returned packageRelativePath "", which should end in ' + + '".metro-package-scope__".', + ); + expect(lookedUpPaths()).toEqual([ + p('/root/src/foo'), + filesIn('/root/src'), + filesIn('/root/src') + '_', + filesIn('/root/src') + '__', + ]); + }); + + test('relative file in the file system root', () => { + useFileMap({[p('/main.js')]: '', [p('/foo.js')]: ''}); + context = {...context, originModulePath: p('/main.js')}; + expect(Resolver.resolve(context, './foo', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/foo.js'), + }); + expect(lookedUpPaths()).toEqual([p('/foo'), filesIn('')]); + }); + + test('relative file with an explicit extension', () => { + expect(Resolver.resolve(context, './foo.js', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/src/foo.js'), + }); + expect(lookedUpPaths()).toEqual([ + p('/root/src/foo.js'), + filesIn('/root/src'), + ]); + }); + + test('relative directory, resolved to its index', () => { + expect(Resolver.resolve(context, './lib', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/src/lib/index.js'), + }); + expect(lookedUpPaths()).toEqual([ + p('/root/src/lib'), + filesIn('/root/src'), + filesIn('/root/src/lib'), + ]); + }); + + test('package entry point', () => { + expect(Resolver.resolve(context, 'pkg', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/node_modules/pkg/lib/index.js'), + }); + // The origin, the package directory, the files in node_modules (`pkg` as + // a file), and the files in the directory of the entry point. + expect(lookedUpPaths()).toEqual([ + p('/root/src/main.js'), + p('/root/node_modules/pkg'), + p('/root/node_modules/pkg'), + filesIn('/root/node_modules'), + filesIn('/root/node_modules/pkg/lib'), + ]); + }); + + test('package subpath', () => { + expect(Resolver.resolve(context, 'pkg/lib/sub', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/node_modules/pkg/lib/sub.js'), + }); + expect(lookedUpPaths()).toEqual([ + p('/root/src/main.js'), + p('/root/node_modules/pkg/lib/sub'), + p('/root/node_modules/pkg/lib/sub'), + filesIn('/root/node_modules/pkg/lib'), + ]); + }); + + test('package subpath redirected by the "browser" field', () => { + expect(Resolver.resolve(context, 'pkg/lib/redirected', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/node_modules/pkg/lib/other.js'), + }); + // The redirected path may be redirected again by its own scope + expect(lookedUpPaths()).toEqual([ + p('/root/src/main.js'), + p('/root/node_modules/pkg/lib/redirected'), + p('/root/node_modules/pkg/lib/other.js'), + filesIn('/root/node_modules/pkg/lib'), + ]); + }); + + test('module under node_modules without a package.json', () => { + expect(Resolver.resolve(context, 'nopkg', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/node_modules/nopkg/index.js'), + }); + expect(lookedUpPaths()).toEqual([ + p('/root/src/main.js'), + p('/root/node_modules/nopkg'), + p('/root/node_modules/nopkg'), + filesIn('/root/node_modules'), + filesIn('/root/node_modules/nopkg'), + ]); + }); + + test('package subpath with package exports', () => { + useFileMap({ + ...fileMap, + [p('/root/node_modules/pkg/package.json')]: JSON.stringify({ + name: 'pkg', + exports: {'./sub': './lib/sub.js'}, + }), + }); + context = {...context, unstable_enablePackageExports: true}; + expect(Resolver.resolve(context, 'pkg/sub', 'ios')).toEqual({ + type: 'sourceFile', + filePath: p('/root/node_modules/pkg/lib/sub.js'), + }); + expect(lookedUpPaths()).toEqual([ + p('/root/src/main.js'), + p('/root/node_modules/pkg/sub'), + p('/root/node_modules/pkg/sub'), + ]); + }); +}); diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index 689958f743..61de009490 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -12,6 +12,7 @@ import type { FileAndDirCandidates, FileCandidates, + PackageForModule, Resolution, ResolutionContext, Result, @@ -31,6 +32,7 @@ import { getPackageEntryPoint, matchSubpathFromMainFields, redirectModulePath, + redirectPackageSubpath, } from './PackageResolve'; import resolveAsset from './resolveAsset'; import isAssetFile from './utils/isAssetFile'; @@ -743,7 +745,12 @@ function resolveFile( const candidateExts: Array = []; const filePathPrefix = path.join(dirPath, fileName); - const sfContext = {...context, candidateExts, filePathPrefix}; + const sfContext = { + ...context, + candidateExts, + filePathPrefix, + candidatePackageScope: getCandidatePackageScope(context, dirPath, fileName), + }; const sourceFileResolution = resolveSourceFile(sfContext, platform); if (sourceFileResolution != null) { if (typeof sourceFileResolution === 'string') { @@ -758,6 +765,9 @@ type SourceFileContext = Readonly<{ ...ResolutionContext, candidateExts: Array, filePathPrefix: string, + // The package scope of every candidate extension, with `packageRelativePath` + // that of `filePathPrefix`, or null if they belong to no package + candidatePackageScope: ?PackageForModule, }>; // Either a full path, or a restricted subset of Resolution. @@ -829,9 +839,17 @@ function resolveSourceFileForExt( extension: string, ): SourceFileResolution { const filePath = `${context.filePathPrefix}${extension}`; + const {candidatePackageScope} = context; const redirectedPath = // Any redirections for the bare path have already happened - extension !== '' ? redirectModulePath(context, filePath) : filePath; + extension !== '' && candidatePackageScope != null + ? redirectPackageSubpath( + context, + filePath, + candidatePackageScope, + candidatePackageScope.packageRelativePath + extension, + ) + : filePath; if (redirectedPath === false) { return {type: 'empty'}; } @@ -843,6 +861,59 @@ function resolveSourceFileForExt( return null; } +// A file name, which need not exist, that stands in for any file in a directory +// when asking for their package scope. While it names a package root, `_` is +// appended, up to MAX_ANY_FILE_NAME_ATTEMPTS names in all. +const ANY_FILE_NAME = '.metro-package-scope'; +const MAX_ANY_FILE_NAME_ATTEMPTS = 3; + +/** + * The package scope of the candidate files `fileName` + extension in `dirPath`, + * with the `packageRelativePath` of `fileName` in `dirPath`. Every file directly + * in a directory has the same package scope, which may differ from the scope of + * the directory, or of a directory named `fileName`: a package root is its own + * scope, and a `node_modules` directory is in the scope containing it, but the + * files directly in it are in none. + */ +function getCandidatePackageScope( + context: ResolutionContext, + dirPath: string, + fileName: string, +): ?PackageForModule { + const dirPrefix = dirPath.endsWith(path.sep) ? dirPath : dirPath + path.sep; + let name = ANY_FILE_NAME; + for (let attempt = 1; ; attempt++) { + const modulePath = dirPrefix + name; + const pkg = context.getPackageForModule(modulePath); + if (pkg == null) { + return null; + } + const {packageRelativePath} = pkg; + const dirRelativeLength = packageRelativePath.length - name.length; + if ( + packageRelativePath.endsWith(name) && + (dirRelativeLength === 0 || + packageRelativePath[dirRelativeLength - 1] === path.sep) + ) { + return { + ...pkg, + packageRelativePath: + packageRelativePath.slice(0, dirRelativeLength) + fileName, + }; + } + // Otherwise this is the scope of something other than the files here, + // usually a package root named `name`. + if (attempt === MAX_ANY_FILE_NAME_ATTEMPTS) { + throw new Error( + `getPackageForModule(${JSON.stringify(modulePath)}) returned ` + + `packageRelativePath ${JSON.stringify(packageRelativePath)}, ` + + `which should end in ${JSON.stringify(name)}.`, + ); + } + name += '_'; + } +} + function isRelativeImport(filePath: string) { return /^[.][.]?(?:[/]|$)/.test(filePath); } diff --git a/packages/metro-resolver/src/types.js b/packages/metro-resolver/src/types.js index d9c2338ca4..fae297e1cd 100644 --- a/packages/metro-resolver/src/types.js +++ b/packages/metro-resolver/src/types.js @@ -167,7 +167,16 @@ export type ResolutionContext = Readonly<{ * for a given absolute candidate path (which need not exist), or null if * there is no package.json closer than the nearest node_modules directory. * - * @deprecated See https://github.com/react/metro/commit/29c77bff31e2475a086bc3f04073f485da8f9ff0 + * A path through a symlink is scoped by where the link is: the walk up + * checks the link target directory, then continues from the link's parent. + * A path that does not exist belongs to the package of its deepest existing + * ancestor, unless the first missing segment is `node_modules`. + * + * To learn the scope of every file in a directory at once, the resolver asks + * about a file named `.metro-package-scope` in it, which usually does not + * exist. + * + * The resolver calls this a few times per resolution, so it must be cheap. */ getPackageForModule: (absoluteModulePath: string) => ?PackageForModule, diff --git a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js index 95e0170e80..6bd5428c56 100644 --- a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js +++ b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js @@ -1371,6 +1371,54 @@ function dep(name: string): TransformResultDependency { filePath: p('/root/node_modules/aPackage/main-custom.js'), }); }); + + // The "browser" field of the package containing a file applies to it. + // These use platform-specific files, which are redirected per candidate + // extension rather than with the extensionless path. + describe('package scope of files reached through a symlink', () => { + test('a node_modules symlink to a package root', async () => { + setMockFileSystem({ + 'index.js': '', + store: { + deps: { + 'package.json': JSON.stringify({ + name: 'deps', + browser: {'./foo.ios.js': './foo-browser.js'}, + }), + 'foo.ios.js': '', + 'foo-browser.js': '', + }, + }, + }); + fs.symlinkSync(p('/root/store/deps'), p('/root/node_modules')); + + resolver = await createResolver({}, 'ios'); + expect(resolver.resolve(p('/root/index.js'), dep('foo'))).toEqual({ + type: 'sourceFile', + filePath: p('/root/store/deps/foo-browser.js'), + }); + }); + + test('a symlink to a node_modules directory', async () => { + setMockFileSystem({ + 'package.json': JSON.stringify({ + name: 'app', + browser: {'./vendor/bar.ios.js': false}, + }), + 'index.js': '', + other: {node_modules: {'bar.ios.js': ''}}, + }); + fs.symlinkSync(p('/root/other/node_modules'), p('/root/vendor')); + + resolver = await createResolver({}, 'ios'); + expect( + resolver.resolve(p('/root/index.js'), dep('./vendor/bar')), + ).toEqual({ + type: 'sourceFile', + filePath: p('/root/other/node_modules/bar.ios.js'), + }); + }); + }); }); describe('platforms', () => { From 88f268451d9eb7d64d3c99fb841b6feef9067c61 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 18 Sep 2026 19:11:19 +0100 Subject: [PATCH 3/5] metro-file-map: Check hierarchicalLookup candidates from their own nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on https://github.com/react/metro/pull/1945. `hierarchicalLookup` walks up from a start path asking whether each ancestor contains a given subpath - it's how `DependencyGraph` finds the closest `package.json`. For every candidate ancestor it joined the candidate's path to the subpath and traversed the result from the root again, even though the traversal that collected the ancestors already holds each candidate's node. The loop even built a `start` argument for `#lookupByNormalPath` to resume from, but it was never read. Each candidate is now checked from its own node: - When the subpath is a single path segment, which is the only way Metro calls it, with one `Map.get`. Whether that applies is decided once per call, by checking the subpath for a path separator. - Otherwise, by traversing only the subpath from the candidate. Expo CLI's `createTypescriptResolver` calls it this way, with deep subpaths such as `node_modules/${targetPath}/tsconfig.json`. Some single segments still take the traversal from the candidate: - A symlinked child, because we need to follow it to learn its type and real path. - A segment on an ancestor of the project root. Its node doesn't hold the segment that leads back down towards the root (with a root of `/A/B/C`, the node for `/A/B` has no `C` entry), so a map lookup there could wrongly report a directory as missing. A subpath that re-enters the project root from an ancestor of it, e.g. `B/C/a/package.json` from `/A`, is still traversed from the root. The joined path collapses to `a/package.json`, of which the candidate's path is no longer a prefix. An empty subpath, `.` and `..` also take the traversal, since a map lookup would answer those differently. The API is unchanged, and observations are recorded exactly as before - the existing test table, which asserts the recorded sets, passes unchanged. This also removes the unused `startPathIdx` and `startNode` options of `#lookupByNormalPath`. I also tried the more explicit alternative of having the caller pass either a `basename` or a `subpath`, which avoids the separator check. It measured 387ns per walk against 394ns for the separator check, within noise, and it changes the call shape for every caller, so it isn't worth it. ## Benchmark Each closest-package walk is 2.1x faster and each deep-subpath walk 1.4x faster, which takes 13% off first-pass resolution time against `main` on our benchmark app (described in the test plan). Resolver output is byte-identical. ### Walks Replaying the closest-package walks of one resolver pass directly against `hierarchicalLookup` (4,483 walks on the benchmark app), once for `package.json` and once for a deep subpath (`node_modules/react/package.json`) from the same start paths. Nanoseconds per walk, medians of 9 interleaved rounds. The middle rows are each half of this diff on its own, to show which half does what: | variant | `package.json` | deep subpath | |---|---|---| | `main` | 911 | 1,150 | | traversal from the candidate only | 598 (−34%) | 792 (−31%) | | `Map.get` for a single segment only | 426 (−53%) | 1,161 (+1%) | | this diff | **426 (−53%)** | **813 (−29%)** | The two halves are complementary: the map lookup is the bigger win for a single segment, and only the traversal from the candidate helps a deep subpath. Together they cost nothing on either path - the ~20ns behind "traversal only" on a deep subpath is the separator check. ### Resolver, end to end Resolution in isolation, 7 interleaved rounds. Only the first pass can move - the later passes are served by `PackageCache` and make no `hierarchicalLookup` calls, and are within ±2% of `main`. | metric | `main` | this diff | |---|---|---| | first pass after startup (ms) | 125.3 | **109.2 (−12.9%)** | ## Changelog Internal ## Test plan ``` yarn jest packages/metro-file-map packages/metro-resolver packages/metro/src/node-haste packages/metro/src/DeltaBundler/__tests__/resolver-test.js yarn flow check yarn typecheck-ts yarn verify-api-snapshots ``` New tests cover a directory-type lookup with a single-segment subpath, a deep subpath, and a subpath that re-enters the project root from an ancestor of it - that last one fails if traversal always starts from the candidate. The existing fixture already covers a symlinked `package.json`, a broken link, and a directory named `package.json`. **Benchmarks** - measured with this diff applied directly to `main` (at a95869a) rather than on top of #1945, so that the baseline is `main`. The ports substitute `main`'s `invalidatedBy` for #1944's observations, and give identical results for every walk, including the recorded paths. **Benchmark app** - a synthetic mid-size RN 0.87 app: 23 common dependencies (Reanimated, React Navigation, TanStack Query, lodash, etc.) and ~1,000 generated first-party modules, 60% of them in a workspace package outside `projectRoot` with its own `node_modules`. The iOS dev bundle is 2.8k modules, from a file map of 34k files. Timings replay the resolutions a real build of it makes against a fresh `DependencyGraph` per process, in interleaved rounds. These timings isolate resolution - no transformation, serialisation or file reads are included. A real build of this app takes around 4s even with a warm transform cache, so resolution is about 2% of it. --- packages/metro-file-map/src/lib/TreeFS.js | 110 ++++++++++++++---- .../src/lib/__tests__/TreeFS-test.js | 65 ++++++++++- 2 files changed, 149 insertions(+), 26 deletions(-) diff --git a/packages/metro-file-map/src/lib/TreeFS.js b/packages/metro-file-map/src/lib/TreeFS.js index 736d2fe375..702e7adc4e 100644 --- a/packages/metro-file-map/src/lib/TreeFS.js +++ b/packages/metro-file-map/src/lib/TreeFS.js @@ -581,9 +581,9 @@ export default class TreeFS implements MutableFileSystem { // traversal, useful when adding files. Will throw if an expected // directory is already present as a file. makeDirectories?: boolean, - startPathIdx?: number, - startNode?: DirectoryNode, - start?: { + // Resume traversal from a known directory node, whose normal path is + // the prefix of the requested path ending at pathIdx - 1. + start?: ?{ ancestorOfRootIdx: ?number, node: DirectoryNode, pathIdx: number, @@ -844,6 +844,10 @@ export default class TreeFS implements MutableFileSystem { * X = dirname(X) * while X !== dirname(X) * + * Each candidate is checked from its own node: a subpath that is a single + * path segment, such as `package.json`, with one map lookup, and a deeper + * subpath by traversing only the subpath. + * * If `observations` is given, records the canonical paths this result * depends upon. * @@ -863,6 +867,12 @@ export default class TreeFS implements MutableFileSystem { absolutePath: string, containerRelativePath: string, } { + // Only a single path segment can be looked up on a candidate's own node. + const isBasename = + subpath !== '' && + subpath !== '.' && + subpath !== '..' && + !subpath.includes(path.sep); const ancestorsOfInput: Array<{ ancestorOfRootIdx: ?number, node: DirectoryNode, @@ -881,7 +891,11 @@ export default class TreeFS implements MutableFileSystem { subpath, opts.subpathType, observations, - null, + { + ancestorOfRootIdx: closestLookup.ancestorOfRootIdx, + node: closestLookup.node, + }, + isBasename, ); if (maybeAbsolutePathMatch != null) { return { @@ -954,14 +968,8 @@ export default class TreeFS implements MutableFileSystem { subpath, opts.subpathType, observations, - { - ancestorOfRootIdx: candidate.ancestorOfRootIdx, - node: candidate.node, - pathIdx: - candidate.normalPath.length > 0 - ? candidate.normalPath.length + 1 - : 0, - }, + candidate, + isBasename, ); if (maybeAbsolutePathMatch != null) { // Determine the input path relative to the current candidate. Note @@ -1001,7 +1009,11 @@ export default class TreeFS implements MutableFileSystem { subpath, opts.subpathType, observations, - null, + { + ancestorOfRootIdx: commonRootDepth + depthBelowCommonRoot, + node: nextNode, + }, + isBasename, ); if (maybeAbsolutePathMatch != null) { const rootDirParts = this.#pathUtils.getParts(); @@ -1030,24 +1042,74 @@ export default class TreeFS implements MutableFileSystem { return null; } + /** + * Whether the real directory at `normalCandidatePath` has `subpath` of the + * given type, returning the absolute real path of the match. + * + * `candidate` is the directory node at `normalCandidatePath`. When + * `isBasename` and the candidate is the root or a descendant of it, the + * child is checked with one map lookup. An ancestor of the root is + * traversed instead, because its node does not hold the segment leading + * back towards the root. Otherwise, traversal starts from the candidate. + */ #checkCandidateHasSubpath( normalCandidatePath: string, subpath: string, subpathType: 'f' | 'd', observations: ?Observations, - start: ?{ - ancestorOfRootIdx: ?number, - node: DirectoryNode, - pathIdx: number, + candidate: { + readonly ancestorOfRootIdx: ?number, + readonly node: DirectoryNode, + ... }, + isBasename: boolean, ): ?string { - const lookupResult = this.#lookupByNormalPath( - this.#pathUtils.joinNormalToRelative(normalCandidatePath, subpath) - .normalPath, - { - observations, - }, - ); + if ( + isBasename && + (candidate.ancestorOfRootIdx == null || candidate.ancestorOfRootIdx === 0) + ) { + const childNode = candidate.node.get(subpath); + // A symlink must be traversed to learn its type and real path, so only + // a missing child, a directory or a regular file is answered here. + if ( + childNode == null || + isDirectory(childNode) || + isRegularFile(childNode) + ) { + const isMatch = + childNode != null && isDirectory(childNode) === (subpathType === 'd'); + if (!isMatch && observations == null) { + return null; + } + const childNormalPath = + normalCandidatePath === '' + ? subpath + : normalCandidatePath + path.sep + subpath; + if (observations) { + observations.existence.add(childNormalPath); + } + return isMatch + ? this.#pathUtils.normalToAbsolute(childNormalPath) + : null; + } + } + const {normalPath: normalSubpath, collapsedSegments} = + this.#pathUtils.joinNormalToRelative(normalCandidatePath, subpath); + const lookupResult = this.#lookupByNormalPath(normalSubpath, { + observations, + // Traverse only the subpath, starting from the candidate node. If the + // join collapsed segments (e.g. '..' + 'root/package.json'), the + // candidate path is no longer a prefix, so start from the root. + start: + collapsedSegments === 0 + ? { + ancestorOfRootIdx: candidate.ancestorOfRootIdx, + node: candidate.node, + pathIdx: + normalCandidatePath === '' ? 0 : normalCandidatePath.length + 1, + } + : null, + }); if ( lookupResult.exists && // Should be a Map iff subpathType is directory diff --git a/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js b/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js index a429a7eb05..ebbbafe5e8 100644 --- a/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js +++ b/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js @@ -528,6 +528,67 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { }); }); + test('finds the closest directory with a single-segment subpath', () => { + const observations = emptyObservations(); + expect( + tfs.hierarchicalLookup( + p('/A/B/C/a/n_m/pkg/subpath/deep'), + 'n_m', + {breakOnSegment: null, subpathType: 'd'}, + observations, + ), + ).toEqual({ + absolutePath: p('/A/B/C/a/n_m/pkg/n_m'), + containerRelativePath: p('subpath/deep'), + }); + expect(observations).toEqual({ + content: new Set(), + existence: new Set([ + p('a/n_m/pkg/subpath/deep/n_m'), + p('a/n_m/pkg/subpath/n_m'), + p('a/n_m/pkg/n_m'), + ]), + }); + }); + + test('finds the closest match for a deep subpath', () => { + const observations = emptyObservations(); + expect( + tfs.hierarchicalLookup( + p('/A/B/C/a/b/c'), + p('n_m/pkg'), + {breakOnSegment: null, subpathType: 'd'}, + observations, + ), + ).toEqual({ + absolutePath: p('/A/B/C/a/n_m/pkg'), + containerRelativePath: p('b/c'), + }); + expect(observations).toEqual({ + content: new Set(), + existence: new Set([p('a/b/c/n_m'), p('a/b/n_m'), p('a/n_m/pkg')]), + }); + }); + + test('finds a subpath re-entering the root from an ancestor of the root', () => { + const observations = emptyObservations(); + expect( + tfs.hierarchicalLookup( + p('/A/foo'), + p('B/C/a/package.json'), + {breakOnSegment: null, subpathType: 'f'}, + observations, + ), + ).toEqual({ + absolutePath: p('/A/B/C/a/package.json'), + containerRelativePath: 'foo', + }); + expect(observations).toEqual({ + content: new Set(), + existence: new Set([p('../../foo'), p('a/package.json')]), + }); + }); + // The fourth column is paths, besides the match itself, whose addition or // removal changes the result, the fifth those whose modification does too - // the symlinks traversed. @@ -662,6 +723,8 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { ) => { const pathMap = (normalPosixPath: string) => mockPathModule.resolve(p('/A/B/C'), p(normalPosixPath)); + const toCanonical = (posixPath: string) => + canonicalTo(p('/A/B/C'))(pathMap(posixPath)); const observations = emptyObservations(); expect( tfs.hierarchicalLookup( @@ -681,8 +744,6 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => { containerRelativePath: p(expectedRelativeSubpath), }, ); - const toCanonical = (posixPath: string) => - canonicalTo(p('/A/B/C'))(pathMap(posixPath)); expect(observations).toEqual({ content: new Set(expectedContent.map(toCanonical)), existence: new Set( From 378980b2eb48eb4645c1cb7111d4126262be55d2 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 25 Sep 2026 19:03:16 +0100 Subject: [PATCH 4/5] Remove PackageCache's per-module closest package memo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on https://github.com/react/metro/pull/1946. ## Summary `PackageCache` remembers the closest `package.json` for every module path it's asked about, with a reverse index from each `package.json` to the module paths under it and a set of paths known to have no package, both of which exist only to invalidate the first. `DependencyGraph` calls `invalidate` for every changed file to keep it honest. With the previous two PRs that memo no longer pays for itself. https://github.com/react/metro/pull/1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and https://github.com/react/metro/pull/1946 made each one a walk of single map lookups, 2.1x faster (870ns -> 406ns). This removes the memo and both indices, and looks up the closest package on every call. The parsed contents of each `package.json` are still cached per path, and `invalidate` now just drops those. The other reason to do this is incremental resolution (https://github.com/react/metro/pull/1944). Any cache that sits between the resolver and the file system has to store the observations that produced each entry and replay them on a hit, or the second resolution to hit the cache can never be invalidated. That's a fair amount of machinery to add to a cache that's worth 3% of first-build resolution time, which is itself a small part of a build. This only makes sense on top of the stack - at #1944, removing the memo costs 14% on a first pass, and nearly doubles a pass that re-resolves everything. \* Described in the test plan. ## Bugs fixed This also fixes two invalidation bugs, both present on main, where the watcher picks up a change on disk but resolution keeps its old answer until Metro restarts: - A nearer `package.json` is added, e.g. creating a nested package in a monorepo. The memo is invalidated through a reverse index from each `package.json` to the modules that resolved to it, and a new `package.json` has no entries, so the modules beneath it keep pointing at the farther one. (It's masked when there was previously no enclosing package at all, which is special-cased.) - A directory symlink is retargeted into another package. Entries are keyed by the path as it was asked for, through the link, and only the link's own path is invalidated. Both changes reset the graph, so everything is re-resolved - against the stale memo, which is probably why they've gone unnoticed. Reproduced with a real `DependencyGraph` and watcher on main and on #1946, using a `browser` redirect in the affected `package.json` to show which one was consulted. With no memo there's nothing to go stale - every lookup walks the live file map. ## Benchmark Resolution time in isolation, replaying every `DependencyGraph.resolveDependency` call made by one iOS dev build of the benchmark app. Nothing is transformed or serialised, so these are percentages of resolution, not of a build. This goes through `DependencyGraph`'s whole-resolution cache, which is cleared only by real file change events, so each row is something that actually happens in a session. 7 interleaved rounds, medians of 15 iterations. Resolver output is byte-identical. | scenario | #1946 | this diff | |---|---|---| | first build (ms) | 92.9 | 95.6 (+2.9%) | | rebuild, nothing changed (ms) | 7.58 | 7.61 | | after a source edit, re-resolving the edited module's 8 dependencies (µs) | 155 | 155 | | after a `package.json` change, re-resolving everything (ms) | 73.6 | 80.4 (+9.3%) | | heap retained after resolving (MB) | 3.0 | **1.9 (−37%)** | The tradeoffs are: - ~3% of resolution time on a first build (3ms here). A real build does far more than resolve - the same build takes around 4s end to end with a warm transform cache - so that's under 0.1% of it. - ~9% (7ms here) when a `package.json` or symlink change resets the whole graph, which is the only time everything is re-resolved against a warm `PackageCache`. Incremental resolution aims to remove those full resets. - Nothing on an ordinary edit. Metro re-resolves only the edited module's dependencies, and the difference there is within noise. ## Test plan ``` yarn jest packages/metro-file-map packages/metro-resolver packages/metro/src/node-haste packages/metro/src/DeltaBundler packages/metro/src/integration_tests yarn flow check yarn verify-api-snapshots ``` `PackageCache-test.js` loses its tests of memo invalidation, along with the structures they tested - with no memo, invalidation is a non-issue by construction. What's left covers the one remaining cache: each `package.json` is parsed once however many modules share it, and is re-read after it's invalidated. **Benchmark app** - a synthetic mid-size RN 0.87 app: 23 common dependencies (Reanimated, React Navigation, TanStack Query, lodash, etc.) and ~1,000 generated first-party modules, 60% of them in a workspace package outside `projectRoot` with its own `node_modules`. The iOS dev bundle is 2.8k modules, from a file map of 34k files. Timings replay the `resolveDependency` calls a real build of it makes against a fresh `DependencyGraph` per process, in interleaved rounds, with source and `package.json` edits made on disk and picked up by the watcher. These timings isolate resolution - no transformation, serialisation or file reads are included. A real build of this app takes around 4s even with a warm transform cache, so resolution is about 2% of it. Changelog: ``` - **[Fix]**: Resolver: Fix stale resolutions after adding a nearer `package.json` or retargeting a symlinked directory, which previously needed a restart ``` --- packages/metro/src/node-haste/PackageCache.js | 85 +--- .../node-haste/__tests__/PackageCache-test.js | 403 +++--------------- 2 files changed, 61 insertions(+), 427 deletions(-) diff --git a/packages/metro/src/node-haste/PackageCache.js b/packages/metro/src/node-haste/PackageCache.js index 8043365bee..58840da46e 100644 --- a/packages/metro/src/node-haste/PackageCache.js +++ b/packages/metro/src/node-haste/PackageCache.js @@ -12,7 +12,7 @@ import type {PackageJson} from 'metro-resolver/private/types'; import {readFileSync} from 'node:fs'; -import {dirname, sep} from 'node:path'; +import {dirname} from 'node:path'; type GetClosestPackageFn = (absoluteFilePath: string) => ?{ packageJsonPath: string, @@ -40,12 +40,6 @@ export class PackageCache { packageJson: PackageJson, }, >; - // Single cache: module path → pre-built result object, or null (no allocation on hit) - #resultByModulePath: Map; - // Reverse index for invalidation: package.json path → set of module paths - #modulePathsByPackagePath: Map>; - // Module paths that resolved to no package.json (null), for invalidation - #modulePathsWithNoPackage: Set; constructor(options: { getClosestPackage: GetClosestPackageFn, @@ -55,9 +49,6 @@ export class PackageCache { this.#getClosestPackage = options.getClosestPackage; this.#readPackageJson = options.readPackageJson ?? readPackageJsonSync; this.#packageCache = new Map(); - this.#resultByModulePath = new Map(); - this.#modulePathsByPackagePath = new Map(); - this.#modulePathsWithNoPackage = new Set(); } getPackage(filePath: string): Readonly<{ @@ -75,86 +66,24 @@ export class PackageCache { return cached; } + /** + * The closest package is looked up on every call rather than remembered per + * module path. Only the parsed contents of each `package.json` are cached. + */ getPackageForModule(absoluteModulePath: string): ?PackageForModule { - const cached = this.#resultByModulePath.get(absoluteModulePath); - - // Distinguish between `null` (positively no closest package) and - // `undefined` (no cached result yet) - // eslint-disable-next-line lint/strictly-null - if (cached !== undefined) { - return cached; - } - const closest = this.#getClosestPackage(absoluteModulePath); if (closest == null) { - this.#resultByModulePath.set(absoluteModulePath, null); - this.#modulePathsWithNoPackage.add(absoluteModulePath); - return null; - } - - const packagePath = closest.packageJsonPath; - - // Track module→package for invalidation - let modulePaths = this.#modulePathsByPackagePath.get(packagePath); - if (modulePaths == null) { - modulePaths = new Set(); - this.#modulePathsByPackagePath.set(packagePath, modulePaths); - } - modulePaths.add(absoluteModulePath); - - const pkg = this.getPackage(packagePath); - if (pkg == null) { return null; } - - // Cache the pre-built result object — no allocation on future hits - const result: PackageForModule = { + const pkg = this.getPackage(closest.packageJsonPath); + return { packageJson: pkg.packageJson, packageRelativePath: closest.packageRelativePath, rootPath: pkg.rootPath, }; - this.#resultByModulePath.set(absoluteModulePath, result); - return result; } invalidate(filePath: string) { this.#packageCache.delete(filePath); - - // Clean up any cached result for this module path (including null). - // Derive the package.json path from the cached result to clean up the - // reverse index. - const cachedResult = this.#resultByModulePath.get(filePath); - this.#resultByModulePath.delete(filePath); - this.#modulePathsWithNoPackage.delete(filePath); - - if (cachedResult != null) { - const packagePath = cachedResult.rootPath + sep + 'package.json'; - const modules = this.#modulePathsByPackagePath.get(packagePath); - if (modules != null) { - modules.delete(filePath); - if (modules.size === 0) { - this.#modulePathsByPackagePath.delete(packagePath); - } - } - } - - // If filePath is a package.json, invalidate all module lookups pointing to it - const modulePaths = this.#modulePathsByPackagePath.get(filePath); - if (modulePaths != null) { - for (const modulePath of modulePaths) { - this.#resultByModulePath.delete(modulePath); - } - this.#modulePathsByPackagePath.delete(filePath); - } - - // If a package.json was created, modified, or deleted, invalidate all - // null-cached module results, since modules that previously had no - // enclosing package.json may now resolve to this one. - if (filePath.endsWith(sep + 'package.json')) { - for (const modulePath of this.#modulePathsWithNoPackage) { - this.#resultByModulePath.delete(modulePath); - } - this.#modulePathsWithNoPackage.clear(); - } } } diff --git a/packages/metro/src/node-haste/__tests__/PackageCache-test.js b/packages/metro/src/node-haste/__tests__/PackageCache-test.js index d83be20744..33f261141e 100644 --- a/packages/metro/src/node-haste/__tests__/PackageCache-test.js +++ b/packages/metro/src/node-haste/__tests__/PackageCache-test.js @@ -39,21 +39,6 @@ function mockPackageJson(filePath: string, json: {name: string, ...}) { }); } -function mockMultiplePackageJsons( - packages: Array<[string, {name: string, ...}]>, -) { - mockReadFileSync.mockImplementation((path, encoding) => { - if (encoding === 'utf8') { - for (const [filePath, json] of packages) { - if (path === filePath) { - return JSON.stringify(json); - } - } - } - throw new Error(`ENOENT: no such file: ${String(path)}`); - }); -} - beforeEach(() => { mockReadFileSync.mockReset(); }); @@ -63,343 +48,63 @@ const PKG_PATH = PKG_ROOT + sep + 'package.json'; const MODULE_A = PKG_ROOT + sep + 'moduleA.js'; const MODULE_B = PKG_ROOT + sep + 'moduleB.js'; -const PKG2_ROOT = sep + ['project', 'lib'].join(sep); -const PKG2_PATH = PKG2_ROOT + sep + 'package.json'; -const MODULE_C = PKG2_ROOT + sep + 'moduleC.js'; - -const MODULE_NO_PKG = sep + ['orphan', 'module.js'].join(sep); - describe('PackageCache', () => { - describe('invalidate', () => { - test('invalidates a package.json and clears module lookups pointing to it', () => { - const closestPackages: ClosestPackageMap = new Map([ - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - [ - MODULE_B, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleB.js'}, - ], - ]); - const cache = createPackageCache(closestPackages); - mockPackageJson(PKG_PATH, {name: 'test-pkg'}); - - // Populate cache - const resultA1 = cache.getPackageForModule(MODULE_A); - const resultB1 = cache.getPackageForModule(MODULE_B); - expect(resultA1?.packageJson.name).toBe('test-pkg'); - expect(resultB1?.packageJson.name).toBe('test-pkg'); - - // Invalidate the package.json - cache.invalidate(PKG_PATH); - - // Update the mock to return new content - mockPackageJson(PKG_PATH, {name: 'updated-pkg'}); - - // Both modules should now return the updated package - const resultA2 = cache.getPackageForModule(MODULE_A); - const resultB2 = cache.getPackageForModule(MODULE_B); - expect(resultA2?.packageJson.name).toBe('updated-pkg'); - expect(resultB2?.packageJson.name).toBe('updated-pkg'); - }); - - test('invalidates a module file and clears its cached result', () => { - const closestPackages: ClosestPackageMap = new Map([ - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - ]); - const cache = createPackageCache(closestPackages); - mockPackageJson(PKG_PATH, {name: 'test-pkg'}); - - // Populate cache - cache.getPackageForModule(MODULE_A); - expect(mockReadFileSync).toHaveBeenCalledTimes(1); - - // Module result is cached - no new reads - cache.getPackageForModule(MODULE_A); - expect(mockReadFileSync).toHaveBeenCalledTimes(1); - - // Invalidate the module - cache.invalidate(MODULE_A); - - // Next lookup should re-resolve - cache.getPackageForModule(MODULE_A); - // package.json is still cached, so no additional readFileSync - expect(mockReadFileSync).toHaveBeenCalledTimes(1); - }); - - test('invalidating a module cleans up the reverse index to prevent memory leaks', () => { - const closestPackages: ClosestPackageMap = new Map([ - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - [ - MODULE_B, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleB.js'}, - ], - ]); - const cache = createPackageCache(closestPackages); - mockPackageJson(PKG_PATH, {name: 'test-pkg'}); - - // Populate both modules - cache.getPackageForModule(MODULE_A); - cache.getPackageForModule(MODULE_B); - - // Invalidate module A - cache.invalidate(MODULE_A); - - // Invalidate the package.json - only module B should need re-resolution - cache.invalidate(PKG_PATH); - mockPackageJson(PKG_PATH, {name: 'updated-pkg'}); - - // Module B should re-resolve to the updated package - const resultB = cache.getPackageForModule(MODULE_B); - expect(resultB?.packageJson.name).toBe('updated-pkg'); - - // Module A should also re-resolve (it was already invalidated) - const resultA = cache.getPackageForModule(MODULE_A); - expect(resultA?.packageJson.name).toBe('updated-pkg'); - }); - - test('null-cached results are invalidated when a new package.json is created', () => { - const closestPackages = new Map< - string, - ?{packageJsonPath: string, packageRelativePath: string}, - >([[MODULE_NO_PKG, null]]); - const cache = createPackageCache(closestPackages); - - // Module resolves to null (no enclosing package.json) - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - - // Cached null is returned - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - - // Simulate creation of a new package.json that now covers this module - const newPkgRoot = sep + 'orphan'; - const newPkgPath = newPkgRoot + sep + 'package.json'; - closestPackages.set(MODULE_NO_PKG, { - packageJsonPath: newPkgPath, - packageRelativePath: 'module.js', - }); - mockPackageJson(newPkgPath, {name: 'new-pkg'}); - - // Invalidate the new package.json (file watcher would trigger this) - cache.invalidate(newPkgPath); - - // Module should now resolve to the new package, not stale null - const result = cache.getPackageForModule(MODULE_NO_PKG); - expect(result?.packageJson.name).toBe('new-pkg'); - expect(result?.packageRelativePath).toBe('module.js'); - }); - - test('null-cached results are not invalidated by non-package.json file changes', () => { - const closestPackages: ClosestPackageMap = new Map< - string, - ?{packageJsonPath: string, packageRelativePath: string}, - >([[MODULE_NO_PKG, null]]); - const cache = createPackageCache(closestPackages); - - // Module resolves to null - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - - // Invalidate an unrelated file (not a package.json) - cache.invalidate(sep + ['orphan', 'other.js'].join(sep)); - - // Null result should still be cached (closestPackages unchanged) - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - }); - - test('invalidating a module with null result cleans up null-tracking set', () => { - const closestPackages = new Map< - string, - ?{packageJsonPath: string, packageRelativePath: string}, - >([ - [MODULE_NO_PKG, null], - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - ]); - const cache = createPackageCache(closestPackages); - mockPackageJson(PKG_PATH, {name: 'test-pkg'}); - - // Populate caches - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - expect(cache.getPackageForModule(MODULE_A)?.packageJson.name).toBe( - 'test-pkg', - ); - - // Invalidate the null-cached module directly - cache.invalidate(MODULE_NO_PKG); - - // Re-resolve - still null since closestPackages hasn't changed - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - }); - - test('modules across different packages are independently invalidated', () => { - const closestPackages: ClosestPackageMap = new Map([ - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - [ - MODULE_C, - {packageJsonPath: PKG2_PATH, packageRelativePath: 'moduleC.js'}, - ], - ]); - const cache = createPackageCache(closestPackages); - mockMultiplePackageJsons([ - [PKG_PATH, {name: 'pkg1'}], - [PKG2_PATH, {name: 'pkg2'}], - ]); - - // Populate cache - expect(cache.getPackageForModule(MODULE_A)?.packageJson.name).toBe( - 'pkg1', - ); - expect(cache.getPackageForModule(MODULE_C)?.packageJson.name).toBe( - 'pkg2', - ); - - // Invalidate only pkg1's package.json - cache.invalidate(PKG_PATH); - mockMultiplePackageJsons([ - [PKG_PATH, {name: 'pkg1-updated'}], - [PKG2_PATH, {name: 'pkg2'}], - ]); - - // Module A should re-resolve to updated pkg1 - expect(cache.getPackageForModule(MODULE_A)?.packageJson.name).toBe( - 'pkg1-updated', - ); - - // Module C should still return cached pkg2 (unchanged) - expect(cache.getPackageForModule(MODULE_C)?.packageJson.name).toBe( - 'pkg2', - ); - }); - - test('package.json deletion invalidates associated modules and null results', () => { - const closestPackages = new Map< - string, - ?{packageJsonPath: string, packageRelativePath: string}, - >([ - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - [MODULE_NO_PKG, null], - ]); - const cache = createPackageCache(closestPackages); - mockPackageJson(PKG_PATH, {name: 'test-pkg'}); - - // Populate both caches - cache.getPackageForModule(MODULE_A); - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - - // Simulate package.json deletion - module A's closest package changes - closestPackages.set(MODULE_A, null); - - // Invalidate the deleted package.json - cache.invalidate(PKG_PATH); - - // Module A should re-resolve (now to null) - expect(cache.getPackageForModule(MODULE_A)).toBe(null); - - // The orphan module's null cache was also cleared (package.json invalidation) - // so it re-resolves, still to null - expect(cache.getPackageForModule(MODULE_NO_PKG)).toBe(null); - }); - - test('invalidating the same file twice is safe', () => { - const closestPackages: ClosestPackageMap = new Map([ - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - ]); - const cache = createPackageCache(closestPackages); - mockPackageJson(PKG_PATH, {name: 'test-pkg'}); - - cache.getPackageForModule(MODULE_A); - - // Double invalidation should not throw - cache.invalidate(MODULE_A); - cache.invalidate(MODULE_A); - - cache.invalidate(PKG_PATH); - cache.invalidate(PKG_PATH); - - mockPackageJson(PKG_PATH, {name: 'updated-pkg'}); - expect(cache.getPackageForModule(MODULE_A)?.packageJson.name).toBe( - 'updated-pkg', - ); - }); - - test('invalidating a file not in any cache is a no-op', () => { - const closestPackages: ClosestPackageMap = new Map([ - [ - MODULE_A, - {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, - ], - ]); - const cache = createPackageCache(closestPackages); - mockPackageJson(PKG_PATH, {name: 'test-pkg'}); - - cache.getPackageForModule(MODULE_A); - - // Invalidating an unrelated file should not affect anything - cache.invalidate(sep + ['unrelated', 'file.js'].join(sep)); - - // Cached result is still valid - const result = cache.getPackageForModule(MODULE_A); - expect(result?.packageJson.name).toBe('test-pkg'); - expect(mockReadFileSync).toHaveBeenCalledTimes(1); - }); - - test('multiple null-cached modules are all invalidated when package.json appears', () => { - const moduleD = sep + ['orphan', 'deep', 'moduleD.js'].join(sep); - const moduleE = sep + ['orphan', 'deep', 'moduleE.js'].join(sep); - const closestPackages: ClosestPackageMap = new Map< - string, - ?{packageJsonPath: string, packageRelativePath: string}, - >([ - [moduleD, null], - [moduleE, null], - ]); - const cache = createPackageCache(closestPackages); - - // Both resolve to null - expect(cache.getPackageForModule(moduleD)).toBe(null); - expect(cache.getPackageForModule(moduleE)).toBe(null); - - // New package.json created - const newPkgRoot = sep + ['orphan', 'deep'].join(sep); - const newPkgPath = newPkgRoot + sep + 'package.json'; - closestPackages.set(moduleD, { - packageJsonPath: newPkgPath, - packageRelativePath: 'moduleD.js', - }); - closestPackages.set(moduleE, { - packageJsonPath: newPkgPath, - packageRelativePath: 'moduleE.js', - }); - mockPackageJson(newPkgPath, {name: 'orphan-pkg'}); + test('reads each package.json once, however many modules it contains', () => { + const closestPackages: ClosestPackageMap = new Map([ + [ + MODULE_A, + {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, + ], + [ + MODULE_B, + {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleB.js'}, + ], + ]); + const cache = createPackageCache(closestPackages); + mockPackageJson(PKG_PATH, {name: 'test-pkg'}); + + expect(cache.getPackageForModule(MODULE_A)?.packageRelativePath).toBe( + 'moduleA.js', + ); + expect(cache.getPackageForModule(MODULE_B)?.packageRelativePath).toBe( + 'moduleB.js', + ); + cache.getPackageForModule(MODULE_A); - cache.invalidate(newPkgPath); + expect(mockReadFileSync).toHaveBeenCalledTimes(1); + }); - // Both modules should now resolve - expect(cache.getPackageForModule(moduleD)?.packageJson.name).toBe( - 'orphan-pkg', - ); - expect(cache.getPackageForModule(moduleE)?.packageJson.name).toBe( - 'orphan-pkg', - ); - }); + test('re-reads an invalidated package.json, for every module in the package', () => { + const closestPackages: ClosestPackageMap = new Map([ + [ + MODULE_A, + {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleA.js'}, + ], + [ + MODULE_B, + {packageJsonPath: PKG_PATH, packageRelativePath: 'moduleB.js'}, + ], + ]); + const cache = createPackageCache(closestPackages); + mockPackageJson(PKG_PATH, {name: 'test-pkg'}); + + // Read and parse the package.json + const resultA1 = cache.getPackageForModule(MODULE_A); + const resultB1 = cache.getPackageForModule(MODULE_B); + expect(resultA1?.packageJson.name).toBe('test-pkg'); + expect(resultB1?.packageJson.name).toBe('test-pkg'); + + // Invalidate the package.json + cache.invalidate(PKG_PATH); + + // Update the mock to return new content + mockPackageJson(PKG_PATH, {name: 'updated-pkg'}); + + // Both modules should now return the updated package + const resultA2 = cache.getPackageForModule(MODULE_A); + const resultB2 = cache.getPackageForModule(MODULE_B); + expect(resultA2?.packageJson.name).toBe('updated-pkg'); + expect(resultB2?.packageJson.name).toBe('updated-pkg'); }); }); From 8226d83507a41d81f5081c9f0a7df712d80d26c4 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 25 Sep 2026 19:03:16 +0100 Subject: [PATCH 5/5] Resolver: Collect file system observations per resolution under unstable_incrementalResolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on https://github.com/react/metro/pull/1948. ## Summary The first consumer of the `Observations` API from https://github.com/react/metro/pull/1944. When `resolver.unstable_incrementalResolution` is enabled, `ModuleResolver.resolveDependency` creates an observations record at the start of each resolution, and everything that resolution looks up in the file map is recorded into it. The record is returned on the `BundlerResolution` as `unstable_observations`. Nothing reads it yet - invalidating from it is the next step - so there's no behaviour change, and with the flag off nothing is allocated. The resolution context given to `metro-resolver` doesn't change. `ModuleResolver` already builds a context per resolution, so when the flag is on it binds the file system capabilities to that resolution's record (`fileSystemLookup: p => fileSystemLookup(p, observations)`), and when it's off it passes the same functions as before. That means a custom `resolveRequest` records whatever it looks up through the context, without knowing about any of this. The observations parameter only appears on Metro's internal option types. What's recorded: - `fileSystemLookup`, and `resolveAsset` through it. - `getPackageForModule`, down to `hierarchicalLookup`. There's no memo in the way since https://github.com/react/metro/pull/1948. - `doesFileExist`. `FileSystem.exists` can't record, so when recording this is answered by `lookup`, which agrees with it - true for exactly the paths found to be a file. Putting the record on the `BundlerResolution` means `DependencyGraph`'s whole-resolution cache replays it for free - every origin given a cached resolution gets its observations too. The one other memo is the empty module, which is resolved once and cached, so a resolution that lands on it returns a copy with the union of what led there and what resolving the empty module observed. Not included, to follow: - The contents of `package.json`. `getPackage` reads with `fs` outside the file map, and Metro has no way to turn an absolute path into a canonical one. The file is recorded in `existence` by `hierarchicalLookup`, so adding or removing one is covered, but editing one isn't. - Haste lookups, which have no bucket yet. - Failed resolutions. They throw, so there's nowhere to put what they observed. ## Benchmark Resolution time in isolation, replaying every `DependencyGraph.resolveDependency` call made by one iOS dev build of the benchmark app (described in the test plan), through the whole-resolution cache. Nothing is transformed or serialised, so these are percentages of resolution, not of a build. 7 interleaved rounds. Resolutions are identical with the flag on and off. | scenario | #1948 | this diff, flag off | this diff, flag on | |---|---|---|---| | first build (ms) | 95.9 | 95.8 (−0.1%) | 111.6 (+16.5%) | | rebuild, nothing changed (ms) | 7.61 | 7.64 (+0.4%) | 7.80 (+2.1%) | | after a source edit, re-resolving the edited module's dependencies (µs) | 153 | 159 (+4.1%) | 180 (+13.2%) | | after a `package.json` change, re-resolving everything (ms) | 79.8 | 80.1 (+0.4%) | 93.1 (+16.2%) | | heap retained after resolving (MB) | | 5.3 | 18.8 (3.5x) | Flag off is within noise of #1948. (The source edit row is 8 resolutions, so that's under a microsecond each, and the runs overlap.) Flag on costs about 16% of resolution time wherever everything is resolved, which is ~2µs per resolution, and 3.5x the retained heap. On the benchmark app that's 100k entries across 6.9k distinct resolutions - a median of 14 each, 90th percentile 27 - naming 28k distinct paths. Nearly all of it is `existence`, and most of that is extension and platform candidates that missed (`a.ios.js`, `a.native.js`, ...), which can't collapse because the directory they're in exists. That's the memory to go after before this is worth turning on, e.g. by recording the directory once instead. ## Test plan ``` yarn jest packages/metro-file-map packages/metro-resolver packages/metro/src/node-haste packages/metro/src/DeltaBundler packages/metro/src/integration_tests yarn flow check yarn typecheck-ts yarn verify-api-snapshots ``` New tests in `resolver-test.js` assert the exact sets recorded for a relative import's candidates, each level probed for a package, a symlink (recorded as `content`, with lookups recorded by real path), asset variants, and a resolution that lands on the empty module. They also check that there's no `unstable_observations` by default, and that origins sharing a cached resolution share its observations. **Benchmark app** - a synthetic mid-size RN 0.87 app: 23 common dependencies (Reanimated, React Navigation, TanStack Query, lodash, etc.) and ~1,000 generated first-party modules, 60% of them in a workspace package outside `projectRoot` with its own `node_modules`. The iOS dev bundle is 2.8k modules, from a file map of 34k files. Timings replay the `resolveDependency` calls a real build of it makes against a fresh `DependencyGraph` per process, in interleaved rounds, with source and `package.json` edits made on disk and picked up by the watcher. A real build of this app takes around 4s even with a warm transform cache, so resolution is about 2% of it. Changelog: Internal --- .../DeltaBundler/__tests__/resolver-test.js | 184 ++++++++++++++++++ packages/metro/src/DeltaBundler/types.js | 12 ++ .../metro/src/node-haste/DependencyGraph.js | 7 +- .../DependencyGraph/ModuleResolution.js | 105 ++++++++-- .../DependencyGraph/createModuleResolver.js | 36 +++- packages/metro/src/node-haste/PackageCache.js | 13 +- 6 files changed, 331 insertions(+), 26 deletions(-) diff --git a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js index 6bd5428c56..7c3086a13b 100644 --- a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js +++ b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js @@ -2897,6 +2897,190 @@ function dep(name: string): TransformResultDependency { }); }); + describe('unstable_incrementalResolution', () => { + const incremental: InputConfigT = { + resolver: {unstable_incrementalResolution: true}, + }; + // Observed paths are canonical: relative to the root, with system + // separators. + const canonical = (...posixPaths: Array) => + new Set(posixPaths.map(posixPath => joinPath(...posixPath.split('/')))); + + test('resolutions carry no observations by default', async () => { + setMockFileSystem({'index.js': '', 'a.js': ''}); + resolver = await createResolver(); + expect( + resolver.resolve(p('/root/index.js'), dep('./a')), + ).not.toHaveProperty('unstable_observations'); + }); + + test('records every candidate probed for a relative import', async () => { + setMockFileSystem({'index.js': '', 'a.js': ''}); + resolver = await createResolver(incremental, 'ios'); + expect(resolver.resolve(p('/root/index.js'), dep('./a'))).toEqual({ + type: 'sourceFile', + filePath: p('/root/a.js'), + unstable_observations: { + // A more specific candidate appearing changes the result + existence: canonical( + 'a', + 'a.ios.js', + 'a.native.js', + 'a.js', + 'package.json', + // The stand-in for the files in the directory, which would + // change their package if it appeared as a package root + '.metro-package-scope', + ), + content: new Set(), + }, + }); + }); + + test('records each level probed for a package', async () => { + setMockFileSystem({ + deep: {dir: {'index.js': ''}}, + node_modules: { + pkg: { + 'package.json': JSON.stringify({name: 'pkg', main: 'main.js'}), + 'main.js': '', + }, + }, + }); + resolver = await createResolver(incremental, 'ios'); + expect( + resolver.resolve(p('/root/deep/dir/index.js'), dep('pkg')), + ).toEqual({ + type: 'sourceFile', + filePath: p('/root/node_modules/pkg/main.js'), + unstable_observations: { + existence: canonical( + // The closest package to the origin + 'deep/dir/index.js', + 'deep/dir/package.json', + 'deep/package.json', + 'package.json', + // Each node_modules on the way up. A missing one is one entry, + // however many packages are looked up beneath it. + 'deep/dir/node_modules', + 'deep/node_modules', + 'node_modules', + // A file is preferred to a package of the same name + 'node_modules/.metro-package-scope', + 'node_modules/pkg.ios.js', + 'node_modules/pkg.native.js', + 'node_modules/pkg.js', + 'node_modules/pkg.ios.json', + 'node_modules/pkg.native.json', + 'node_modules/pkg.json', + 'node_modules/pkg', + 'node_modules/pkg/package.json', + 'node_modules/pkg/.metro-package-scope', + 'node_modules/pkg/main.js', + ), + content: new Set(), + }, + }); + }); + + test('records a traversed symlink as content, and lookups by real path', async () => { + setMockFileSystem({'index.js': '', real: {'x.js': ''}}); + fs.symlinkSync(p('/root/real'), p('/root/link')); + resolver = await createResolver(incremental, 'ios'); + expect(resolver.resolve(p('/root/index.js'), dep('./link/x'))).toEqual({ + type: 'sourceFile', + filePath: p('/root/real/x.js'), + unstable_observations: { + existence: canonical( + 'real/x', + 'real/x.ios.js', + 'real/x.native.js', + 'real/x.js', + 'real/.metro-package-scope', + 'real/package.json', + 'package.json', + ), + // Retargeting the link changes what every path through it means + content: canonical('link'), + }, + }); + }); + + test('records every asset variant probed', async () => { + setMockFileSystem({ + 'index.js': '', + 'asset.png': '', + 'asset@2x.png': '', + }); + resolver = await createResolver(incremental, 'ios'); + expect( + resolver.resolve(p('/root/index.js'), dep('./asset.png')) + .unstable_observations, + ).toEqual({ + existence: canonical( + 'asset.png', + 'asset@1x.png', + 'asset@1.5x.png', + 'asset@2x.png', + 'asset@3x.png', + 'asset@4x.png', + 'package.json', + ), + content: new Set(), + }); + }); + + test('a resolution landing on the empty module includes what resolving the empty module observed', async () => { + setMockFileSystem({ + 'empty.js': '', + withBrowser: { + 'package.json': JSON.stringify({ + name: 'with-browser', + browser: {'./gone.js': false}, + }), + 'index.js': '', + 'gone.js': '', + }, + }); + resolver = await createResolver( + { + resolver: { + emptyModulePath: p('/root/empty.js'), + unstable_incrementalResolution: true, + }, + }, + 'ios', + ); + expect( + resolver.resolve(p('/root/withBrowser/index.js'), dep('./gone')), + ).toEqual({ + type: 'sourceFile', + filePath: p('/root/empty.js'), + unstable_observations: { + existence: canonical( + // What led to the empty module + 'withBrowser/gone', + 'withBrowser/package.json', + // What resolving the empty module observed + 'empty.js', + '.metro-package-scope', + 'package.json', + ), + content: new Set(), + }, + }); + }); + + test('origins sharing a cached resolution share its observations', async () => { + setMockFileSystem({'index.js': '', 'other.js': '', 'a.js': ''}); + resolver = await createResolver(incremental, 'ios'); + const first = resolver.resolve(p('/root/index.js'), dep('./a')); + const second = resolver.resolve(p('/root/other.js'), dep('./a')); + expect(second).toBe(first); + expect(second.unstable_observations?.existence.size).toBeGreaterThan(0); + }); + }); + describe('schemeResolvers', () => { test('config schemeResolvers are applied to scheme-prefixed specifiers', async () => { setMockFileSystem({'index.js': '', 'a.js': ''}); diff --git a/packages/metro/src/DeltaBundler/types.js b/packages/metro/src/DeltaBundler/types.js index ac000fa07a..d114a6dc21 100644 --- a/packages/metro/src/DeltaBundler/types.js +++ b/packages/metro/src/DeltaBundler/types.js @@ -151,9 +151,21 @@ export type AllowOptionalDependenciesWithOptions = { export type AllowOptionalDependencies = boolean | AllowOptionalDependenciesWithOptions; +// What a resolution observed of the file system: canonical paths whose +// addition or removal can change its result, and those whose modification +// can too. Satisfies `metro-file-map`'s `Observations`, which populates it. +export type ResolutionObservations = Readonly<{ + existence: Set, + content: Set, +}>; + export type BundlerResolution = Readonly<{ type: 'sourceFile', filePath: string, + // What this resolution observed of the file system, if + // `resolver.unstable_incrementalResolution` is enabled. It is shared by + // every caller given this resolution, so must not be mutated. + unstable_observations?: ResolutionObservations, }>; export type Options = Readonly<{ diff --git a/packages/metro/src/node-haste/DependencyGraph.js b/packages/metro/src/node-haste/DependencyGraph.js index ee4a63f708..8d5cf1b549 100644 --- a/packages/metro/src/node-haste/DependencyGraph.js +++ b/packages/metro/src/node-haste/DependencyGraph.js @@ -11,6 +11,7 @@ import type { BundlerResolution, + ResolutionObservations, TransformResultDependency, } from '../DeltaBundler/types'; import type {DynamicRoot} from '../lib/dynamicRoots'; @@ -135,8 +136,8 @@ export default class DependencyGraph extends EventEmitter { ); this._resolutionCache = new Map(); this.#packageCache = new PackageCache({ - getClosestPackage: absoluteModulePath => - this._getClosestPackage(absoluteModulePath), + getClosestPackage: (absoluteModulePath, observations) => + this._getClosestPackage(absoluteModulePath, observations), }); this._createModuleResolver(); }); @@ -193,6 +194,7 @@ export default class DependencyGraph extends EventEmitter { _getClosestPackage( absoluteModulePath: string, + observations?: ?ResolutionObservations, ): ?{packageJsonPath: string, packageRelativePath: string} { const result = this._fileSystem.hierarchicalLookup( absoluteModulePath, @@ -201,6 +203,7 @@ export default class DependencyGraph extends EventEmitter { breakOnSegment: 'node_modules', subpathType: 'f', }, + observations, ); return result ? { diff --git a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js index d48a00125d..305cf3dd86 100644 --- a/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js +++ b/packages/metro/src/node-haste/DependencyGraph/ModuleResolution.js @@ -11,13 +11,13 @@ import type { BundlerResolution, + ResolutionObservations, TransformResultDependency, } from '../../DeltaBundler/types'; import type {Reporter} from '../../lib/reporting'; import type {ResolverInputOptions} from '../../shared/types'; import type { CustomResolver, - DoesFileExist, FileCandidates, FileSystemLookup, Resolution, @@ -36,20 +36,34 @@ import util from 'node:util'; type Options = Readonly<{ assetExts: ReadonlySet, disableHierarchicalLookup: boolean, - doesFileExist: DoesFileExist, + doesFileExist: ( + filePath: string, + observations?: ?ResolutionObservations, + ) => boolean, emptyModulePath: string, extraNodeModules: ?Object, - fileSystemLookup: FileSystemLookup, + fileSystemLookup: ( + filePath: string, + observations?: ?ResolutionObservations, + ) => ReturnType, getHasteModulePath: (name: string, platform: ?string) => ?string, getHastePackagePath: (name: string, platform: ?string) => ?string, mainFields: ReadonlyArray, getPackage: (packageJsonPath: string) => ?PackageJson, - getPackageForModule: (absolutePath: string) => ?PackageForModule, + getPackageForModule: ( + absolutePath: string, + observations?: ?ResolutionObservations, + ) => ?PackageForModule, nodeModulesPaths: ReadonlyArray, preferNativePlatform: boolean, projectRoot: string, reporter: Reporter, - resolveAsset: ResolveAsset, + resolveAsset: ( + dirPath: string, + assetName: string, + extension: string, + observations?: ?ResolutionObservations, + ) => ReturnType, resolveRequest: ?CustomResolver, schemeResolvers: Readonly<{[scheme: string]: CustomResolver}>, sourceExts: ReadonlyArray, @@ -61,6 +75,12 @@ type Options = Readonly<{ unstable_incrementalResolution: boolean, }>; +// Every record is created here, with the same properties in the same order, +// so that reading them stays monomorphic on the lookup hot path. +function createObservations(): ResolutionObservations { + return {existence: new Set(), content: new Set()}; +} + export class ModuleResolver { _options: Options; // A module representing the project root, used as the origin when resolving `emptyModulePath`. @@ -125,6 +145,14 @@ export class ModuleResolver { unstable_incrementalResolution, } = this._options; + // Everything this resolution observes of the file system is recorded here. + // The capabilities given to the resolver are bound to it, so that the + // resolution context keeps its shape and a custom resolver records what + // it looks up without having to know about it. + const observations: ?ResolutionObservations = unstable_incrementalResolution + ? createObservations() + : null; + try { const result = Resolver.resolve( createDefaultContext( @@ -134,17 +162,31 @@ export class ModuleResolver { customResolverOptions: resolverOptions.customResolverOptions ?? {}, dev: resolverOptions.dev, disableHierarchicalLookup, - doesFileExist, + doesFileExist: + observations == null + ? doesFileExist + : filePath => doesFileExist(filePath, observations), extraNodeModules, - fileSystemLookup, + fileSystemLookup: + observations == null + ? fileSystemLookup + : filePath => fileSystemLookup(filePath, observations), getPackage, - getPackageForModule, + getPackageForModule: + observations == null + ? getPackageForModule + : absolutePath => + getPackageForModule(absolutePath, observations), isESMImport: dependency.data.isESMImport, mainFields, nodeModulesPaths, originModulePath, preferNativePlatform, - resolveAsset, + resolveAsset: + observations == null + ? resolveAsset + : (dirPath, assetName, extension) => + resolveAsset(dirPath, assetName, extension, observations), resolveHasteModule: (name: string) => this._options.getHasteModulePath(name, platform), resolveHastePackage: (name: string) => @@ -163,7 +205,7 @@ export class ModuleResolver { dependency.name, platform, ); - return this._getFileResolvedModule(result); + return this._getFileResolvedModule(result, observations); } catch (error) { if (error instanceof Resolver.FailedToResolvePathError) { const {candidates} = error; @@ -229,18 +271,53 @@ export class ModuleResolver { /** * TODO: Return Resolution instead of coercing to BundlerResolution here */ - _getFileResolvedModule(resolution: Resolution): BundlerResolution { + _getFileResolvedModule( + resolution: Resolution, + observations: ?ResolutionObservations, + ): BundlerResolution { switch (resolution.type) { case 'sourceFile': - return resolution; + return observations == null + ? resolution + : { + filePath: resolution.filePath, + type: 'sourceFile', + unstable_observations: observations, + }; 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 {filePath: arbitrary, type: 'sourceFile'}; + return observations == null + ? {filePath: arbitrary, type: 'sourceFile'} + : { + filePath: arbitrary, + type: 'sourceFile', + unstable_observations: observations, + }; case 'empty': - return this._getEmptyModule(); + const emptyModule = this._getEmptyModule(); + if (observations == null) { + return emptyModule; + } + // The empty module is resolved once and cached, so a resolution that + // lands on it depends on what that resolution observed as well as on + // what led here. + const emptyObservations = emptyModule.unstable_observations; + if (emptyObservations != null) { + for (const canonicalPath of emptyObservations.existence) { + observations.existence.add(canonicalPath); + } + for (const canonicalPath of emptyObservations.content) { + observations.content.add(canonicalPath); + } + } + return { + filePath: emptyModule.filePath, + type: 'sourceFile', + unstable_observations: observations, + }; case 'virtualModule': // Reserved for future implementation. throw new Error('Virtual modules are not yet implemented.'); diff --git a/packages/metro/src/node-haste/DependencyGraph/createModuleResolver.js b/packages/metro/src/node-haste/DependencyGraph/createModuleResolver.js index fa2db68a69..85efa30ab4 100644 --- a/packages/metro/src/node-haste/DependencyGraph/createModuleResolver.js +++ b/packages/metro/src/node-haste/DependencyGraph/createModuleResolver.js @@ -9,6 +9,7 @@ * @oncall react_native */ +import type {ResolutionObservations} from '../../DeltaBundler/types'; import type {PackageCache} from '../PackageCache'; import type {ConfigT} from 'metro-config'; import type {FileSystem, HasteMap} from 'metro-file-map'; @@ -31,8 +32,11 @@ export default function createModuleResolver({ hasteMap, packageCache, }: CreateModuleResolverOptions): ModuleResolver { - const fileSystemLookup = (filePath: string): ReturnType => { - const result = fileSystem.lookup(filePath); + const fileSystemLookup = ( + filePath: string, + observations?: ?ResolutionObservations, + ): ReturnType => { + const result = fileSystem.lookup(filePath, observations); if (result.exists) { return { exists: true, @@ -46,7 +50,18 @@ export default function createModuleResolver({ return new ModuleResolver({ assetExts: new Set(config.resolver.assetExts), disableHierarchicalLookup: config.resolver.disableHierarchicalLookup, - doesFileExist: (filePath: string) => fileSystem.exists(filePath), + doesFileExist: ( + filePath: string, + observations?: ?ResolutionObservations, + ) => { + if (observations == null) { + return fileSystem.exists(filePath); + } + // `exists` cannot record what it observed, and is true for exactly the + // paths that `lookup` finds to be a file. + const result = fileSystem.lookup(filePath, observations); + return result.exists && result.type === 'f'; + }, emptyModulePath: config.resolver.emptyModulePath, extraNodeModules: config.resolver.extraNodeModules, fileSystemLookup, @@ -62,14 +77,21 @@ export default function createModuleResolver({ return null; } }, - getPackageForModule: (absolutePath: string) => - packageCache.getPackageForModule(absolutePath), + getPackageForModule: ( + absolutePath: string, + observations?: ?ResolutionObservations, + ) => packageCache.getPackageForModule(absolutePath, observations), mainFields: config.resolver.resolverMainFields, nodeModulesPaths: config.resolver.nodeModulesPaths, preferNativePlatform: true, projectRoot: config.projectRoot, reporter: config.reporter, - resolveAsset: (dirPath: string, assetName: string, extension: string) => { + resolveAsset: ( + dirPath: string, + assetName: string, + extension: string, + observations?: ?ResolutionObservations, + ) => { const basePath = dirPath + path.sep + assetName; const assets = [ basePath + extension, @@ -77,7 +99,7 @@ export default function createModuleResolver({ resolution => basePath + '@' + resolution + 'x' + extension, ), ] - .map(assetPath => fileSystemLookup(assetPath).realPath) + .map(assetPath => fileSystemLookup(assetPath, observations).realPath) .filter(Boolean); return assets.length ? assets : null; diff --git a/packages/metro/src/node-haste/PackageCache.js b/packages/metro/src/node-haste/PackageCache.js index 58840da46e..c4fcf806a9 100644 --- a/packages/metro/src/node-haste/PackageCache.js +++ b/packages/metro/src/node-haste/PackageCache.js @@ -9,12 +9,16 @@ * @oncall react_native */ +import type {ResolutionObservations} from '../DeltaBundler/types'; import type {PackageJson} from 'metro-resolver/private/types'; import {readFileSync} from 'node:fs'; import {dirname} from 'node:path'; -type GetClosestPackageFn = (absoluteFilePath: string) => ?{ +type GetClosestPackageFn = ( + absoluteFilePath: string, + observations?: ?ResolutionObservations, +) => ?{ packageJsonPath: string, packageRelativePath: string, }; @@ -70,8 +74,11 @@ export class PackageCache { * The closest package is looked up on every call rather than remembered per * module path. Only the parsed contents of each `package.json` are cached. */ - getPackageForModule(absoluteModulePath: string): ?PackageForModule { - const closest = this.#getClosestPackage(absoluteModulePath); + getPackageForModule( + absoluteModulePath: string, + observations?: ?ResolutionObservations, + ): ?PackageForModule { + const closest = this.#getClosestPackage(absoluteModulePath, observations); if (closest == null) { return null; }