Copy PNPM patches to isolated directory #145
Conversation
When includePatchedDependencies is enabled, copy relevant patch files from the workspace root to the isolated directory and update the isolated package.json with patch references. Patches are filtered based on the target package's dependencies, respecting the includeDevDependencies configuration option.
There was a problem hiding this comment.
Pull request overview
This PR implements support for copying PNPM patch files to isolated package directories when the includePatchedDependencies configuration option is enabled. The implementation filters patches based on the target package's dependencies, ensuring only relevant patches are copied and referenced in both the isolated package.json and lockfile.
Key Changes
- Added
patchedDependenciesfield to thePackageManifesttype definition to support PNPM patch configuration - Implemented patch file copying with dependency-based filtering that respects production and dev dependency boundaries
- Updated lockfile generation to filter
patchedDependenciesentries to match only packages present in the isolated environment
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/lib/types.ts | Extended PackageManifest type with optional pnpm.patchedDependencies field |
| src/lib/patches/copy-patches.ts | New module implementing patch file copying with filtering logic based on target dependencies |
| src/lib/lockfile/helpers/generate-pnpm-lockfile.ts | Added patch filtering for lockfile generation, replacing previous comment about incomplete implementation |
| src/isolate.ts | Integrated patch copying into isolation workflow and updated isolated manifest with patch references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use getIsolateRelativeLogPath for patches directory logging - Add getIsolateRelativeLogPath to test mocks - Format CLAUDE.md code style section
The lockfile was being generated before patches were copied, causing paths in the lockfile to not match the actual copied files. Changes: - Reorder operations so copyPatches runs before processLockfile - Update copyPatches to return PatchFile format with path and hash - Read hashes from original lockfile to preserve them after copying - Pass pre-computed patched dependencies to lockfile generator - Add PatchFile interface to types.ts - Update tests to expect PatchFile format
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Patches are now automatically copied if they exist in the workspace. There's no need for an explicit opt-in setting.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Instead of flattening all patches to patches/ with collision avoidance, preserve the original directory structure when copying patch files. This eliminates unnecessary complexity: - Remove collision detection and filename renaming logic - Paths in patchedDependencies remain unchanged from workspace root - Simpler code, no edge cases for naming conflicts
Patched dependencies are a pnpm-specific feature. Skip copying patches when forceNpm is enabled or when the detected package manager is not pnpm.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When
includePatchedDependenciesis enabled, copy relevant patch files from the workspace root to the isolated directory and update the isolatedpackage.jsonwith patch references.Patches are filtered based on the target package's dependencies:
includeDevDependenciesis trueThe lockfile generator also filters
patchedDependenciesto match only relevant packages.Based on #142 but adapted for the updated tooling (tsdown instead of tsup).
Subsequent improvements
After the initial implementation, several refinements were made to improve code quality, fix bugs, and ensure correctness:
Code organization
Extracted shared utilities - The
getPackageNamefunction (which parses package specs like@firebase/[email protected]into package names) was duplicated betweencopy-patches.tsandfilter-patched-dependencies.ts. Extracted tosrc/lib/utils/get-package-name.ts.Extracted filtering logic - The patch filtering logic was also duplicated. Extracted to
src/lib/utils/filter-patched-dependencies.tsas a generic utility that works with any value type via TypeScript generics.Preserved folder structure - Patch files are copied preserving their original directory structure rather than flattening to a single
patches/directory.Bug fixes
Manifest consistency - The filtering was using the original
targetPackageManifestinstead of the adaptedoutputManifest. This could cause issues since the output manifest may have different dependency references. Fixed to useoutputManifestfor filtering.Lockfile patch paths - The lockfile was being generated before patches were copied, causing a mismatch between the paths in the lockfile and the actual copied files. Fixed by:
copyPatchesruns beforeprocessLockfilecopyPatchesreturn the fullPatchFileformat (withpathandhash)Test coverage
Added comprehensive unit tests for the new utilities:
get-package-name.test.ts- 14 tests covering scoped packages, edge cases, and malformed inputfilter-patched-dependencies.test.ts- 11 tests for filtering logic with various dependency configurationscopy-patches.test.ts- 10 tests for the patch copying functionalityType safety
PatchFileinterface ({ path: string; hash: string }) matching the pnpm lockfile format, since the installed@pnpm/typesversion doesn't export this typefilterPatchedDependencies<T>to preserve the value type through filtering operations