perf: always link runtimes from the global virtual store directory#10233
perf: always link runtimes from the global virtual store directory#10233
Conversation
5d59c3e to
2366e1e
Compare
54f8a02 to
12e5107
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a performance optimization where runtime dependencies (Node.js, Bun, Deno) are always linked from the global virtual store directory, enabling sharing across projects regardless of the enableGlobalVirtualStore setting. This change adds a new required globalVirtualStoreDir parameter to multiple interfaces across the codebase.
Key changes:
- Added
isRuntimeDepPathutility function to identify runtime dependencies by their@runtime:prefix - Introduced
globalVirtualStoreDirparameter to dependency resolution and installation options - Modified dependency graph building to conditionally place runtime dependencies in the global virtual store
- Extracted
calcGraphNodeHashfunction fromiterateHashedGraphNodesfor reuse in runtime dependency handling
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/dependency-path/src/index.ts |
Added isRuntimeDepPath function to detect runtime dependencies using regex pattern |
packages/dependency-path/test/index.ts |
Added unit tests for the new isRuntimeDepPath function |
packages/calc-dep-state/src/index.ts |
Extracted calcGraphNodeHash function and exported DepsStateCache type for external use |
deps/graph-builder/src/iteratePkgsForVirtualStore.ts |
Updated to use globalVirtualStoreDir for runtime dependencies and refactored directory path construction |
deps/graph-builder/src/lockfileToDepGraph.ts |
Added globalVirtualStoreDir to options interface and updated variable name for clarity |
pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts |
Added globalVirtualStoreDir to ResolveDependenciesOptions interface |
pkg-manager/resolve-dependencies/src/index.ts |
Modified extendGraph to process runtime dependencies; missing proper globalVirtualStoreDir usage |
pkg-manager/headless/src/index.ts |
Added globalVirtualStoreDir to HeadlessOptions interface |
pkg-manager/core/src/install/extendInstallOptions.ts |
Added logic to set globalVirtualStoreDir based on store configuration; has redundant assignment |
pkg-manager/core/src/install/index.ts |
Threaded globalVirtualStoreDir parameter through to resolveDependencies |
pkg-manager/core/src/getPeerDependencyIssues.ts |
Added globalVirtualStoreDir requirement and passed through to resolution |
pkg-manager/core/test/install/nodeRuntime.ts |
Added assertions verifying runtime dependencies link to global virtual store |
.changeset/angry-streets-bow.md |
Documents the major version bump for all affected packages |
💡 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.
| test('isRuntimeDepPath', () => { | ||
| expect(isRuntimeDepPath('node@runtime:20.1.0' as DepPath)).toBeTruthy() | ||
| expect(isRuntimeDepPath('node@20.1.0' as DepPath)).toBeFalsy() | ||
| }) |
There was a problem hiding this comment.
The test for isRuntimeDepPath only covers Node.js runtime, but the implementation (via the regex /^(?:node|bun|deno)@runtime:/) also supports bun and deno runtimes. Consider adding test cases for these runtimes as well:
expect(isRuntimeDepPath('bun@runtime:1.0.0' as DepPath)).toBeTruthy()
expect(isRuntimeDepPath('deno@runtime:1.30.0' as DepPath)).toBeTruthy()|
Pardon my ignorance, but what are 'runtime dependencies'? Are those node/bun/deno? and this PR we treat them specially? Thanks~ |
|
yes, those are node/bun/deno. This PR will make installing them faster because they will be just symlinked from a shared location. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Runtime dependencies (node/deno/bun) are big and they don't have any dependencies of their own. It makes sense to symlink them from a single location on the disk (the global virtual store).