fix: apply patches during pnpm fetch behind skipped file deps#11063
Open
teee32 wants to merge 3 commits into
Open
fix: apply patches during pnpm fetch behind skipped file deps#11063teee32 wants to merge 3 commits into
teee32 wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an edge case where pnpm fetch imports patched packages into the virtual store but fails to actually apply the patch when the package is only reachable through skipped local file: dependencies.
Changes:
- Extend the
buildModules()root set inheadlessInstall()to include all dependency graph nodes that havepatchinfo. - Add a regression test covering a patched registry dependency that is only reachable via a skipped
file:dependency duringfetch. - Add a changeset documenting the user-facing
pnpm fetchfix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| installing/deps-restorer/src/index.ts | Ensures patched packages are always reachable by the patch/build traversal even when direct importer roots are incomplete (e.g. fetch skipping file: deps). |
| installing/commands/test/fetch.ts | Adds a regression test to assert patched content is present after pnpm fetch in the skipped-file: scenario. |
| .changeset/fair-mails-wave.md | Publishes the behavior change as a patch release for the affected packages/CLI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ensure fetch still traverses patched packages when the package tree is only reachable through local file dependencies that are skipped during fetch.
Locate the patched package directory by probing for the actual file instead of matching on the directory name prefix, which can fail on Windows due to virtualStoreDirMaxLength truncation.
6af1951 to
504c192
Compare
Move the fix from deps-restorer (adding patched nodes to directNodes) to deps/graph-builder where the problem originates. When local file: deps are skipped during fetch, their transitive registry children become orphaned in the build traversal. Now lockfileToDepGraph promotes those children into directDependenciesByImporterId so they're naturally reachable.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4 tasks
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.
Problem
pnpm fetchcan import a registry package into the virtual store without applying its patch when that package is only reachable through a localfile:dependency.Reproduction / Observation
A minimal repro looks like this:
file:./local-pkg.local-pkgdepends onis-positive@1.0.0.patchedDependenciescontains a patch foris-positive@1.0.0.pnpm install --lockfile-only, remove the root manifest, then runpnpm fetch.Before this change,
fetchcreated the patched virtual-store directory (is-positive@1.0.0_patch_hash=...) but the importedindex.jsstill contained the unpatched source.Root cause
headlessInstall()runs patch application viabuildModules(), but its root set is built fromdirectDependenciesByImporterId.When
fetchskips localfile:dependencies, packages that are only reachable through those skipped nodes are no longer part of that root set. They still get imported into the virtual store from the lockfile, but the later patch/build traversal never visits them.Change
buildModules()root set ininstalling/deps-restorer.file:dependency.pnpm fetchbehavior fix.Why this fix
This keeps the fix narrow:
patch != nullare added as extra build rootsfetch+ skipped-local-dependency pathRisk
Low. The change does not alter dependency selection or lockfile handling; it only ensures the existing patch step can reach already-imported patched packages.
Validation
pnpm --dir _work/pnpm --filter @pnpm/installing.commands run test test/fetch.tspnpm --dir _work/pnpm --filter @pnpm/installing.deps-installer run test test/install/patch.ts -t 'patch package with exact version'Fixes #11034.