Preserve patches for transitive dependencies via internal packages#177
Merged
Preserve patches for transitive dependencies via internal packages#177
Conversation
When a workspace has pnpm.patchedDependencies for a package that is not a direct dependency of the isolate target but is reachable through an internal workspace package, the patch was silently dropped. filterPatchedDependencies now also accepts a set of dependency names reachable via internal workspace manifests, computed by the new collectReachablePackageNames helper. Patches for names in this set are preserved so pnpm can apply them at install time.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 883753b. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an isolation bug where workspace-level pnpm.patchedDependencies entries targeting transitives introduced via internal workspace packages were omitted from the isolated output.
Changes:
- Extend
filterPatchedDependenciesto preserve patches for package names reachable via internal workspace packages. - Add
collectReachablePackageNamesto compute reachable dependency names by walking the target + internal workspace manifests. - Plumb
packagesRegistryintocopyPatchesso it can compute reachability and pass it into the filter, with regression tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/utils/filter-patched-dependencies.ts | Adds reachableDependencyNames fallback when filtering patched deps. |
| src/lib/utils/filter-patched-dependencies.test.ts | Adds tests for reachability-based inclusion/exclusion behavior. |
| src/lib/registry/index.ts | Re-exports the new reachability collector. |
| src/lib/registry/collect-reachable-package-names.ts | New helper to walk target + internal workspace manifests and collect reachable dep names. |
| src/lib/registry/collect-reachable-package-names.test.ts | Unit tests for reachability collection (direct deps, recursion, cycles, devDep rules). |
| src/lib/patches/copy-patches.ts | Computes reachable names and passes them into filterPatchedDependencies; adds packagesRegistry param. |
| src/lib/patches/copy-patches.test.ts | Updates tests for the new packagesRegistry arg + regression coverage for issue #167 plumbing. |
| src/isolate.ts | Passes packagesRegistry into copyPatches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address three issues raised by review bots on PR #177: - filterPatchedDependencies: when a patch target is listed in the target's devDependencies with includeDevDependencies=false, still check the reachable set before excluding. A package can be both a target devDep and a prod transitive via an internal workspace package, in which case the patch still applies. - collectReachablePackageNames: also walk optionalDependencies and peerDependencies. Both can lead to packages being installed in the isolate (pnpm installs peers by default via autoInstallPeers), so patches on them should be preserved. - Replace the test that codified the buggy devDep-over-reachable behavior with one covering the correct prod-transitive override case, and add tests for optional/peer deps.
This was referenced Apr 30, 2026
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.
Fixes #167.
When a workspace declares
pnpm.patchedDependenciesfor a package that is not a direct dependency of the isolate target but is reachable through an internal workspace package, the patch was silently dropped from the isolated output.filterPatchedDependencieschecked only the target's owndependencies/devDependencies, so e.g. a patch ontslib(a dep of an internalfirebase-package) never made it into the isolatedpackage.jsonor thepatches/folder.copyPatchesnow computes the set of dependency names reachable from the target by walking the target manifest plus the manifests of every internal workspace package it can reach. That set is passed tofilterPatchedDependenciesas a third matching fallback after direct prod and direct dev deps: if the patch's package name is in the reachable set, the patch is preserved. Internal packages'devDependenciesare intentionally not followed since they aren't installed in the isolate.Limitation: this does not cover patches on deeper external-to-external transitives (target → externalA → externalB-patched). Addressing those requires lockfile-based pruning, which hasn't been observed as a practical need yet.
Scope:
packages (isolate-package)Visibility: user-facing