Repository navigation
Conversation
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
…one map lookup Stacked on #1945. ## Summary `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 `start` argument the loop built for this was never read.) This lets the caller give either `opts.subpath`, which may be deep and behaves as before, or `opts.basename`, a single path segment. A basename is checked with one `Map.get` on the candidate's own node. It isn't validated to be a single segment - that'd be a cost on every call for something the caller already knows. With a basename, two cases still traverse: - A symlinked child, because we need to follow it to learn its type and real path. - Ancestors of the project root. Their nodes don'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. Observations are recorded exactly as before. The existing test table now runs every row both as a `basename` and as a `subpath`, and the two must agree on the result and on the recorded sets. ### Compatibility The subpath moves from a positional argument into `opts`, so this changes the call shape for anything calling `hierarchicalLookup`. Metro's only caller passes `package.json`. Expo CLI's `createTypescriptResolver` passes deep subpaths (`node_modules/${targetPath}/tsconfig.json`) - that currently runs against `@expo/metro-file-map`'s own port of `TreeFS`, so it isn't affected, and deep subpaths remain supported here via `opts.subpath`. ## Benchmark Same harness as #1944 (6,935 resolutions, 7 interleaved rounds). Resolver output is byte-identical. | metric | #1945 | this diff | |---|---|---| | first pass after startup (ms) | 85.9 | **81.8 (−4.8%)** | | warm pass (ms) | 54.0 | 54.0 | | after a source change (ms) | 61.6 | 61.8 (+0.3%) | | after a `package.json` change (ms) | 64.4 | 63.7 (−1.1%) | Only the first pass moves, which is expected - after that these lookups are served by `PackageCache`. Isolating the walks themselves, by replaying the 4,483 closest-package walks of one pass directly against `hierarchicalLookup` (9 interleaved rounds): | | per walk | | |---|---|---| | #1945 | 865ns | | | this diff, `basename: 'package.json'` | **387ns** | **−55%** | | alternative: API unchanged, take the fast path when `subpath` has no separator | 394ns | −54% | The alternative is within noise of this, so the choice between them is about the API rather than speed. A deep subpath (`node_modules/react/package.json` from the same start paths) is 1,083ns -> 1,120ns per walk with either, about 3%, for the extra branch per candidate. ## 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 by basename, and a deep subpath. The existing fixture already covers a symlinked `package.json`, a broken link, and a directory named `package.json`. Changelog: Internal
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
…ith one map lookup Stacked on #1945. ## Summary `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 `start` argument the loop built for this was never read.) When the subpath is a single path segment, which is the only way Metro calls it, each candidate is now checked with one `Map.get` on its own node. The API is unchanged, and a deep subpath behaves exactly as before. Whether the fast path applies is decided once per call, by checking the subpath for a path separator. With a single segment, two cases still traverse: - A symlinked child, because we need to follow it to learn its type and real path. - Ancestors of the project root. Their nodes don'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. An empty subpath, `.` and `..` also take the traversal, since a map lookup would answer those differently. Observations are recorded exactly as before - the existing test table, which asserts the recorded sets, passes unchanged. 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 this, within noise, and it changes the call shape for every caller - including Expo CLI's `createTypescriptResolver`, which passes deep subpaths - so it isn't worth it. ## Benchmark Same harness as #1944 (6,935 resolutions, 7 interleaved rounds). Resolver output is byte-identical. | metric | #1945 | this diff | |---|---|---| | first pass after startup (ms) | 85.2 | **82.2 (−3.5%)** | | warm pass (ms) | 53.3 | 54.8 | | after a source change (ms) | 61.1 | 62.3 | | after a `package.json` change (ms) | 64.2 | 64.2 | Only the first pass can move. The other passes make no `hierarchicalLookup` calls at all, because they're served by `PackageCache`, so those rows are run-to-run noise. Isolating the walks themselves, by replaying the 4,483 closest-package walks of one pass directly against `hierarchicalLookup`: 870ns -> 406ns per walk (−53%). A deep subpath (`node_modules/react/package.json` from the same start paths) is 1,084ns -> 1,111ns, for the extra branch per candidate. ## 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, and a deep subpath. The existing fixture already covers a symlinked `package.json`, a broken link, and a directory named `package.json`. Changelog: Internal
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
September 18, 2026 18:11
b28de70 to
4acaaa7
Compare
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
…ith one map lookup Stacked on #1945. ## Summary `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 `start` argument the loop built for this was never read.) When the subpath is a single path segment, which is the only way Metro calls it, each candidate is now checked with one `Map.get` on its own node. The API is unchanged, and a deep subpath behaves exactly as before. Whether the fast path applies is decided once per call, by checking the subpath for a path separator. With a single segment, two cases still traverse: - A symlinked child, because we need to follow it to learn its type and real path. - Ancestors of the project root. Their nodes don'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. An empty subpath, `.` and `..` also take the traversal, since a map lookup would answer those differently. Observations are recorded exactly as before - the existing test table, which asserts the recorded sets, passes unchanged. 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 this, within noise, and it changes the call shape for every caller - including Expo CLI's `createTypescriptResolver`, which passes deep subpaths - so it isn't worth it. ## Benchmark Same harness as #1944 (6,935 resolutions, 7 interleaved rounds). Resolver output is byte-identical. | metric | #1945 | this diff | |---|---|---| | first pass after startup (ms) | 85.2 | **82.2 (−3.5%)** | | warm pass (ms) | 53.3 | 54.8 | | after a source change (ms) | 61.1 | 62.3 | | after a `package.json` change (ms) | 64.2 | 64.2 | Only the first pass can move. The other passes make no `hierarchicalLookup` calls at all, because they're served by `PackageCache`, so those rows are run-to-run noise. Isolating the walks themselves, by replaying the 4,483 closest-package walks of one pass directly against `hierarchicalLookup`: 870ns -> 406ns per walk (−53%). A deep subpath (`node_modules/react/package.json` from the same start paths) is 1,084ns -> 1,111ns, for the extra branch per candidate. ## 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, and a deep subpath. The existing fixture already covers a symlinked `package.json`, a broken link, and a directory named `package.json`. Changelog: Internal
robhogan
added this pull request to stack #1947
September 18, 2026 18:12
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
September 18, 2026 18:20
4acaaa7 to
19f8d76
Compare
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
…ith one map lookup Stacked on #1945. ## Summary `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 `start` argument the loop built for this was never read.) When the subpath is a single path segment, which is the only way Metro calls it, each candidate is now checked with one `Map.get` on its own node. The API is unchanged, and a deep subpath behaves exactly as before. Whether the fast path applies is decided once per call, by checking the subpath for a path separator. With a single segment, two cases still traverse: - A symlinked child, because we need to follow it to learn its type and real path. - Ancestors of the project root. Their nodes don'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. An empty subpath, `.` and `..` also take the traversal, since a map lookup would answer those differently. Observations are recorded exactly as before - the existing test table, which asserts the recorded sets, passes unchanged. 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 this, within noise, and it changes the call shape for every caller - including Expo CLI's `createTypescriptResolver`, which passes deep subpaths - so it isn't worth it. ## Benchmark Each closest-package walk is 2.1x faster, which takes 3.5% off first-pass resolution time on our benchmark app (described in the test plan). 7 interleaved rounds. Resolver output is byte-identical. | metric | #1945 | this diff | |---|---|---| | first pass after startup (ms) | 85.2 | **82.2 (−3.5%)** | | warm pass (ms) | 53.3 | 54.8 | | after a source change (ms) | 61.1 | 62.3 | | after a `package.json` change (ms) | 64.2 | 64.2 | Only the first pass can move. The other passes make no `hierarchicalLookup` calls at all, because they're served by `PackageCache`, so those rows are run-to-run noise. Isolating the walks themselves, by replaying the closest-package walks of one pass directly against `hierarchicalLookup`: 870ns -> 406ns per walk (2.1x). A deep subpath (`node_modules/react/package.json` from the same start paths) is 1,084ns -> 1,111ns, for the extra branch per candidate. ## 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, and a deep subpath. The existing fixture already covers a symlinked `package.json`, a broken link, and a directory named `package.json`. **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. Changelog: Internal
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
Stacked on #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. #1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and #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 (#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. 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. ## Benchmark Replaying every `DependencyGraph.resolveDependency` call made by one iOS dev build of the benchmark app. 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). - ~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` is restructured around the new contract: the closest package is looked up on every call, a change of closest package is reflected with no invalidation, each `package.json` is parsed once however many modules share it, and invalidating one re-reads it. Tests of the reverse index and the no-package set go with those structures. **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. Changelog: Internal
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
September 18, 2026 18:27
19f8d76 to
727ba55
Compare
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
…ith one map lookup Stacked on #1945. ## Summary `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 `start` argument the loop built for this was never read.) When the subpath is a single path segment, which is the only way Metro calls it, each candidate is now checked with one `Map.get` on its own node. The API is unchanged, and a deep subpath behaves exactly as before. Whether the fast path applies is decided once per call, by checking the subpath for a path separator. With a single segment, two cases still traverse: - A symlinked child, because we need to follow it to learn its type and real path. - Ancestors of the project root. Their nodes don'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. An empty subpath, `.` and `..` also take the traversal, since a map lookup would answer those differently. Observations are recorded exactly as before - the existing test table, which asserts the recorded sets, passes unchanged. 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 this, within noise, and it changes the call shape for every caller - including Expo CLI's `createTypescriptResolver`, which passes deep subpaths - so it isn't worth it. ## Benchmark Each closest-package walk is 2.1x faster, which takes 3.5% 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 | #1945 | this diff | |---|---|---| | first pass after startup (ms) | 85.2 | **82.2 (−3.5%)** | | warm pass (ms) | 53.3 | 54.8 | | after a source change (ms) | 61.1 | 62.3 | | after a `package.json` change (ms) | 64.2 | 64.2 | Only the first pass can move. The other passes make no `hierarchicalLookup` calls at all, because they're served by `PackageCache`, so those rows are run-to-run noise. Isolating the walks themselves, by replaying the closest-package walks of one pass directly against `hierarchicalLookup`: 870ns -> 406ns per walk (2.1x). A deep subpath (`node_modules/react/package.json` from the same start paths) is 1,084ns -> 1,111ns, for the extra branch per candidate. ## 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, and a deep subpath. The existing fixture already covers a symlinked `package.json`, a broken link, and a directory named `package.json`. **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
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
Stacked on #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. #1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and #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 (#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. ## 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` is restructured around the new contract: the closest package is looked up on every call, a change of closest package is reflected with no invalidation, each `package.json` is parsed once however many modules share it, and invalidating one re-reads it. Tests of the reverse index and the no-package set go with those structures. **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: Internal
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
Stacked on #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. #1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and #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 (#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` is restructured around the new contract: the closest package is looked up on every call, a change of closest package is reflected with no invalidation, each `package.json` is parsed once however many modules share it, and invalidating one re-reads it. Tests of the reverse index and the no-package set go with those structures. **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 ```
robhogan
added a commit
that referenced
this pull request
Sep 18, 2026
Stacked on #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. #1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and #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 (#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 ```
robhogan
added a commit
that referenced
this pull request
Sep 25, 2026
Stacked on #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.
robhogan
added a commit
that referenced
this pull request
Sep 25, 2026
Stacked on #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. #1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and #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 (#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 ```
This was referenced Sep 26, 2026
Resolver: Collect file system observations per resolution under unstable_incrementalResolution
#1949
Draft
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
September 29, 2026 14:30
727ba55 to
dbb3bab
Compare
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
September 29, 2026 14:36
dbb3bab to
5e264cd
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The lexical node_modules shortcut changes package redirection behavior for symlinked directories.
3 open findings
What changed in this PR
Refactors resolver package-scope lookup to reuse one directory lookup across extension candidates, improving resolution performance.
Changes:
- Shares package scope across source-file candidates.
- Extracts package-subpath redirection logic.
- Adds lookup-count tests and resets test caches.
| File | Description |
|---|---|
packages/metro-resolver/src/resolve.js |
Reuses directory package scope. |
packages/metro-resolver/src/PackageResolve.js |
Extracts redirection helper. |
packages/metro-resolver/src/types.js |
Clarifies package lookup semantics. |
packages/metro-resolver/src/__tests__/package-scope-test.js |
Tests package-scope lookup counts. |
packages/metro/src/DeltaBundler/__tests__/resolver-test.js |
Resets test file-map cache. |
packages/metro/src/integration_tests/metro.config.js |
Resets integration-test cache. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+113
to
+115
| // Tests that create a second resolver must not read the first's file map | ||
| // cache: `v8.deserialize` runs outside the Jest sandbox, so a cached tree | ||
| // holds `Map`s from another realm that fail TreeFS's `instanceof` checks. |
Comment on lines
+22
to
+26
| // The file map cache defaults to `os.tmpdir()` and is keyed only by build | ||
| // parameters, so without this every test file, and every run, reads whatever | ||
| // the previous one wrote. A cache read inside Jest also holds `Map`s from | ||
| // another realm (`v8.deserialize` runs outside the sandbox) that fail | ||
| // TreeFS's `instanceof` checks. |
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
3 times, most recently
from
October 9, 2026 15:12
84730aa to
d56f1cf
Compare
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
October 9, 2026 15:25
d56f1cf to
206e12b
Compare
robhogan
removed this pull request from stack #1947
October 9, 2026 15:33
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
October 9, 2026 15:34
206e12b to
b512700
Compare
robhogan
changed the base branch from
main
to
robhogan/redirect-package-subpath
October 9, 2026 15:34
robhogan
added this pull request to stack #2045
October 9, 2026 15:34
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
October 9, 2026 15:42
b512700 to
7e3174d
Compare
| ...context, | ||
| candidateExts, | ||
| filePathPrefix, | ||
| candidatePackageScope: getCandidatePackageScope(context, dirPath, fileName), |
Comment on lines
+906
to
+910
| 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)}.`, |
robhogan
added a commit
that referenced
this pull request
Oct 14, 2026
## 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. #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
…andidates Stacked on #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
robhogan
force-pushed
the
robhogan/resolve-package-once-per-directory
branch
from
October 14, 2026 10:11
7e3174d to
56dedec
Compare
robhogan
added a commit
that referenced
this pull request
Oct 14, 2026
Stacked on #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. #1945 cut closest-package lookups 3.7x (64k -> 17k per build on our mid-size RN benchmark app*), and #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 (#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 ```
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



An internal resolver refactor that improves resolution performance by ~30% and unblocks removing lookup caching, which unblocks incremental resolution and on-demand FS.
Context
When resolving a file, the resolver checks for
package.jsonredirections once per candidate extension, asking for the package scope of each candidate path in turn:metro/packages/metro-resolver/src/resolve.js
Lines 827 to 834 in 6dbe979
All extension candidates in a directory have the same package scope, but we look each one up separately. And even though
PackageCachememoises lookups it does so by module path and only caches hits, and since most candidates are files that don't exist, each one misses the first time.This is the first of three PRs cutting the cost of closest-package lookups, before observations are collected per resolution in #1949.
This change
This looks up the package of the directory once and shares it across the candidates. Resolver output is identical.
It cuts package scope lookups 3.7x (64k -> 17k per pass on our benchmark app) and takes 28% off first-pass resolution time (120ms -> 86ms). Warm passes are within a few percent, where
PackageCachewas already hitting.Also removes the
@deprecatedtag fromgetPackageForModule, since the resolver depends on it.Benchmark
AI-driven, 7 interleaved rounds, resolution measured in isolation.
getPackageForModulecalls per passpackage.jsonchange (ms)Changelog: Internal
Test plan
New unit test pins the number of
getPackageForModulecalls for the common specifier shapes.Benchmark app (AI-driven harness, used throughout this stack): a synthetic RN 0.87 app with 23 common dependencies and ~1,000 generated first-party modules, 60% of them in a workspace package outside
projectRootwith its ownnode_modules. 2.8k modules in the iOS dev bundle, 34k files in the file map. Timings replay the resolutions of a real build against a freshDependencyGraphand isolate resolution. A full build of this app takes ~4s with a warm transform cache, so resolution is ~2% of it.