From 45b4275093be44f5e0bd9c5a2ef16cc9ec967122 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 18 Sep 2026 19:11:19 +0100 Subject: [PATCH] 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(