From 4e24402f1e1b2448447a700851cbab86b263af9e Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 18 Sep 2026 19:51:00 +0100 Subject: [PATCH] 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 | 177 ++++++++++++++++++ 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, 324 insertions(+), 26 deletions(-) diff --git a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js index 8083401968..7042ec83a8 100644 --- a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js +++ b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js @@ -2853,6 +2853,183 @@ 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', + ), + 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/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/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/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', + '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 6c297bfea8..76f4e44b30 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 {ResolverInputOptions} from '../shared/types'; @@ -130,8 +131,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(); }); @@ -175,6 +176,7 @@ export default class DependencyGraph extends EventEmitter { _getClosestPackage( absoluteModulePath: string, + observations?: ?ResolutionObservations, ): ?{packageJsonPath: string, packageRelativePath: string} { const result = this._fileSystem.hierarchicalLookup( absoluteModulePath, @@ -183,6 +185,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 fef3b7fa63..e48fbcb54d 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; }