From 394d88cf5eb674be9c51ba34d79237b7086a48ee Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 27 Dec 2025 12:21:19 +0100 Subject: [PATCH] feat: enable injected local packages to work with global virtual store (#10366) * feat: enable injected local packages to work with global virtual store by leveraging `pkgLocationsByDepPath` for `file:` dependencies. * fix: populate `pkgLocationsByDepPath` directly for directory dependencies in the graph builder * refactor: store directory dependencies as a Map instead of an object * refactor: improve file: dependency target directory resolution by prioritizing `directoryDepsByDepPath` and providing a lockfile fallback. * refactor: remove `pkgLocationsByDepPath` from hoisted dependency graph generation parameters * test: fix * test: fix * refactor: simplify directory lookup for injected workspace packages by directly using the dependency graph * refactor: move extendProjectsWithTargetDirs to headless module and update imports * refactor: make `directoryDepsByDepPath` required in `LockfileToDepGraphOptions` and remove its nullish coalescing in headless * refactor: directory dependency tracking by renaming `directoryDepsByDepPath` to `injectionTargetsByDepPath` and extracting related logic, and remove an unused export. * docs: add changesets * fix: implemented CR suggestions --- .changeset/brave-bears-worry.md | 5 ++ .changeset/fix-injected-deps-global-vstore.md | 9 ++ .changeset/spotty-gifts-chew.md | 5 ++ deps/graph-builder/src/lockfileToDepGraph.ts | 17 +++- .../utils/src/extendProjectsWithTargetDirs.ts | 36 -------- lockfile/utils/src/index.ts | 1 - pkg-manager/core/src/install/index.ts | 33 ++++++-- .../core/test/install/globalVirtualStore.ts | 84 ++++++++++++++++++- .../src/extendProjectsWithTargetDirs.ts | 26 ++++++ pkg-manager/headless/src/index.ts | 11 +-- .../headless/src/lockfileToHoistedDepGraph.ts | 15 +++- 11 files changed, 184 insertions(+), 58 deletions(-) create mode 100644 .changeset/brave-bears-worry.md create mode 100644 .changeset/fix-injected-deps-global-vstore.md create mode 100644 .changeset/spotty-gifts-chew.md delete mode 100644 lockfile/utils/src/extendProjectsWithTargetDirs.ts create mode 100644 pkg-manager/headless/src/extendProjectsWithTargetDirs.ts diff --git a/.changeset/brave-bears-worry.md b/.changeset/brave-bears-worry.md new file mode 100644 index 0000000000..4edafac2f1 --- /dev/null +++ b/.changeset/brave-bears-worry.md @@ -0,0 +1,5 @@ +--- +"@pnpm/headless": minor +--- + +Export extendProjectsWithTargetDirs. diff --git a/.changeset/fix-injected-deps-global-vstore.md b/.changeset/fix-injected-deps-global-vstore.md new file mode 100644 index 0000000000..a78cd8fb02 --- /dev/null +++ b/.changeset/fix-injected-deps-global-vstore.md @@ -0,0 +1,9 @@ +--- +"@pnpm/deps.graph-builder": patch +"@pnpm/headless": patch +"@pnpm/core": patch +--- + +Fixed injected local packages to work correctly with the global virtual store [#10366](https://github.com/pnpm/pnpm/pull/10366). + +When using `nodeLinker: 'isolated'` with `enableGlobalVirtualStore: true`, injected workspace packages now use the correct hash-based paths from the global virtual store instead of project-relative paths. diff --git a/.changeset/spotty-gifts-chew.md b/.changeset/spotty-gifts-chew.md new file mode 100644 index 0000000000..7da131a9ed --- /dev/null +++ b/.changeset/spotty-gifts-chew.md @@ -0,0 +1,5 @@ +--- +"@pnpm/lockfile.utils": major +--- + +Remove extendProjectsWithTargetDirs. diff --git a/deps/graph-builder/src/lockfileToDepGraph.ts b/deps/graph-builder/src/lockfileToDepGraph.ts index 72079b2745..e1eb4e0032 100644 --- a/deps/graph-builder/src/lockfileToDepGraph.ts +++ b/deps/graph-builder/src/lockfileToDepGraph.ts @@ -99,7 +99,7 @@ export interface LockfileToDepGraphResult { hoistedLocations?: Record symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId prevGraph?: DependenciesGraph - pkgLocationsByDepPath?: Record + injectionTargetsByDepPath: Map } /** @@ -119,6 +119,7 @@ export async function lockfileToDepGraph ( const { graph, locationByDepPath, + injectionTargetsByDepPath, } = await buildGraphFromPackages(lockfile, currentLockfile, opts) const _getChildrenPaths = getChildrenPaths.bind(null, { @@ -156,7 +157,7 @@ export async function lockfileToDepGraph ( directDependenciesByImporterId[importerId] = _getChildrenPaths(rootDeps, null, importerId) } - return { graph, directDependenciesByImporterId } + return { graph, directDependenciesByImporterId, injectionTargetsByDepPath } } async function buildGraphFromPackages ( @@ -166,10 +167,13 @@ async function buildGraphFromPackages ( ): Promise<{ graph: DependenciesGraph locationByDepPath: Record + injectionTargetsByDepPath: Map }> { const currentPackages = currentLockfile?.packages ?? {} const graph: DependenciesGraph = {} const locationByDepPath: Record = {} + // Only populated for directory deps (injected workspace packages) + const injectionTargetsByDepPath = new Map() const _getPatchInfo = getPatchInfo.bind(null, opts.patchedDependencies) const promises: Array> = [] @@ -201,7 +205,8 @@ async function buildGraphFromPackages ( return } - const depIsPresent = !('directory' in pkgSnapshot.resolution && pkgSnapshot.resolution.directory != null) && + const isDirectoryDep = 'directory' in pkgSnapshot.resolution && pkgSnapshot.resolution.directory != null + const depIsPresent = !isDirectoryDep && currentPackages[depPath] && equals(currentPackages[depPath].dependencies, pkgSnapshot.dependencies) @@ -210,6 +215,10 @@ async function buildGraphFromPackages ( const modules = path.join(dirInVirtualStore, 'node_modules') const dir = path.join(modules, pkgName) locationByDepPath[depPath] = dir + // Track directory deps for injected workspace packages + if (isDirectoryDep) { + injectionTargetsByDepPath.set(depPath, [dir]) + } let dirExists: boolean | undefined if ( @@ -273,7 +282,7 @@ async function buildGraphFromPackages ( })()) } await Promise.all(promises) - return { graph, locationByDepPath } + return { graph, locationByDepPath, injectionTargetsByDepPath } } interface GetChildrenPathsContext { diff --git a/lockfile/utils/src/extendProjectsWithTargetDirs.ts b/lockfile/utils/src/extendProjectsWithTargetDirs.ts deleted file mode 100644 index a450263c79..0000000000 --- a/lockfile/utils/src/extendProjectsWithTargetDirs.ts +++ /dev/null @@ -1,36 +0,0 @@ -import path from 'path' -import { type LockfileObject, type TarballResolution } from '@pnpm/lockfile.types' -import { depPathToFilename } from '@pnpm/dependency-path' -import { type ProjectId, type DepPath } from '@pnpm/types' -import { packageIdFromSnapshot } from './packageIdFromSnapshot.js' -import { nameVerFromPkgSnapshot } from './nameVerFromPkgSnapshot.js' - -type GetLocalLocations = (depPath: DepPath, pkgName: string) => string[] - -export function extendProjectsWithTargetDirs ( - projects: Array, - lockfile: LockfileObject, - ctx: { - virtualStoreDir: string - pkgLocationsByDepPath?: Record - virtualStoreDirMaxLength: number - } -): Array { - const getLocalLocations: GetLocalLocations = ctx.pkgLocationsByDepPath != null - ? (depPath: DepPath) => ctx.pkgLocationsByDepPath![depPath] - : (depPath: DepPath, pkgName: string) => [path.join(ctx.virtualStoreDir, depPathToFilename(depPath, ctx.virtualStoreDirMaxLength), 'node_modules', pkgName)] - const projectsById: Record = - Object.fromEntries(projects.map((project) => [project.id, { ...project, targetDirs: [] as string[] }])) - for (const [depPath, pkg] of Object.entries(lockfile.packages ?? {})) { - if ((pkg.resolution as TarballResolution)?.type !== 'directory') continue - const pkgId = packageIdFromSnapshot(depPath as DepPath, pkg) - const { name: pkgName } = nameVerFromPkgSnapshot(depPath, pkg) - const importerId = pkgId.replace(/^file:/, '') as ProjectId - if (projectsById[importerId] == null) continue - const localLocations = getLocalLocations(depPath as DepPath, pkgName) - if (!localLocations) continue - projectsById[importerId].targetDirs.push(...localLocations) - projectsById[importerId].stages = ['preinstall', 'install', 'postinstall', 'prepare', 'prepublishOnly'] - } - return Object.values(projectsById) as Array -} diff --git a/lockfile/utils/src/index.ts b/lockfile/utils/src/index.ts index fbb63826b9..9597b1eb83 100644 --- a/lockfile/utils/src/index.ts +++ b/lockfile/utils/src/index.ts @@ -1,6 +1,5 @@ import { refToRelative } from '@pnpm/dependency-path' -export { extendProjectsWithTargetDirs } from './extendProjectsWithTargetDirs.js' export { nameVerFromPkgSnapshot } from './nameVerFromPkgSnapshot.js' export { packageIdFromSnapshot } from './packageIdFromSnapshot.js' export { packageIsIndependent } from './packageIsIndependent.js' diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index 1de936f007..9bf580740d 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -25,7 +25,7 @@ import { } from '@pnpm/lockfile.settings-checker' import { PnpmError } from '@pnpm/error' import { getContext, type PnpmContext } from '@pnpm/get-context' -import { headlessInstall, type InstallationResultStats } from '@pnpm/headless' +import { extendProjectsWithTargetDirs, headlessInstall, type InstallationResultStats } from '@pnpm/headless' import { makeNodeRequireOption, runLifecycleHook, @@ -43,7 +43,6 @@ import { type CatalogSnapshots, } from '@pnpm/lockfile.fs' import { writePnpFile } from '@pnpm/lockfile-to-pnp' -import { extendProjectsWithTargetDirs } from '@pnpm/lockfile.utils' import { allProjectsAreUpToDate, satisfiesPackageManifest } from '@pnpm/lockfile.verification' import { getPreferredVersionsFromLockfileAndManifests } from '@pnpm/lockfile.preferred-versions' import { logger, globalInfo, streamParser } from '@pnpm/logger' @@ -1488,10 +1487,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { } })) - const projectsWithTargetDirs = extendProjectsWithTargetDirs(projects, newLockfile, { - virtualStoreDir: ctx.virtualStoreDir, - virtualStoreDirMaxLength: opts.virtualStoreDirMaxLength, - }) + const projectsWithTargetDirs = getProjectsWithTargetDirs(projects, newLockfile, dependenciesGraph) const currentLockfileDir = path.join(ctx.rootModulesDir, '.pnpm') await Promise.all([ opts.useLockfile && opts.saveLockfile @@ -1749,3 +1745,28 @@ export class IgnoredBuildsError extends PnpmError { function dedupePackageNamesFromIgnoredBuilds (ignoredBuilds: IgnoredBuilds): string[] { return Array.from(new Set(Array.from(ignoredBuilds ?? []).map(dp.removeSuffix))).sort(lexCompare) } + +/** + * Build injectionTargetsByDepPath from the dependenciesGraph for injected workspace packages + * and extend projects with their target directories. + * The dependenciesGraph already has the correct `dir` values after `extendGraph` is applied + * (which uses the correct hash-based paths when global virtual store is enabled). + */ +function getProjectsWithTargetDirs ( + projects: T[], + lockfile: LockfileObject, + dependenciesGraph: DependenciesGraph +): Array { + const injectionTargetsByDepPath = new Map() + if (lockfile.packages) { + for (const [depPath, { resolution }] of Object.entries(lockfile.packages)) { + if (resolution?.type === 'directory') { + const graphNode = dependenciesGraph[depPath as DepPath] + if (graphNode?.dir) { + injectionTargetsByDepPath.set(depPath, [graphNode.dir]) + } + } + } + } + return extendProjectsWithTargetDirs(projects, injectionTargetsByDepPath) +} diff --git a/pkg-manager/core/test/install/globalVirtualStore.ts b/pkg-manager/core/test/install/globalVirtualStore.ts index ac480971bd..933eff006c 100644 --- a/pkg-manager/core/test/install/globalVirtualStore.ts +++ b/pkg-manager/core/test/install/globalVirtualStore.ts @@ -1,7 +1,9 @@ import fs from 'fs' import path from 'path' -import { prepareEmpty } from '@pnpm/prepare' -import { install } from '@pnpm/core' +import { assertProject } from '@pnpm/assert-project' +import { prepareEmpty, preparePackages } from '@pnpm/prepare' +import { install, type MutatedProject, mutateModules, type ProjectOptions } from '@pnpm/core' +import { type ProjectRootDir } from '@pnpm/types' import { sync as rimraf } from '@zkochan/rimraf' import { testDefaults } from '../utils/index.js' @@ -71,3 +73,81 @@ test('modules are correctly updated when using a global virtual store', async () expect(fs.existsSync(path.join(globalVirtualStoreDir, '@pnpm.e2e/peer-c/2.0.0', files[0], 'node_modules/@pnpm.e2e/peer-c/package.json'))).toBeTruthy() } }) + +test('injected local packages work with global virtual store', async () => { + const project1Manifest = { + name: 'project-1', + version: '1.0.0', + dependencies: { + 'is-positive': '1.0.0', + }, + } + const project2Manifest = { + name: 'project-2', + version: '1.0.0', + dependencies: { + 'project-1': 'workspace:1.0.0', + }, + dependenciesMeta: { + 'project-1': { + injected: true, + }, + }, + } + preparePackages([ + { + location: 'project-1', + package: project1Manifest, + }, + { + location: 'project-2', + package: project2Manifest, + }, + ]) + fs.writeFileSync('project-1/foo.js', '', 'utf8') + + const globalVirtualStoreDir = path.resolve('links') + const importers: MutatedProject[] = [ + { + mutation: 'install', + rootDir: path.resolve('project-1') as ProjectRootDir, + }, + { + mutation: 'install', + rootDir: path.resolve('project-2') as ProjectRootDir, + }, + ] + const allProjects: ProjectOptions[] = [ + { + buildIndex: 0, + manifest: project1Manifest, + rootDir: path.resolve('project-1') as ProjectRootDir, + }, + { + buildIndex: 0, + manifest: project2Manifest, + rootDir: path.resolve('project-2') as ProjectRootDir, + }, + ] + + await mutateModules(importers, testDefaults({ + autoInstallPeers: false, + allProjects, + enableGlobalVirtualStore: true, + dedupeInjectedDeps: false, + virtualStoreDir: globalVirtualStoreDir, + })) + + // Verify project-2 has project-1 installed + expect(fs.existsSync(path.resolve('project-2/node_modules/project-1'))).toBeTruthy() + + // Verify the modules manifest has injectedDeps pointing to global virtual store + const rootModules = assertProject(process.cwd()) + const modulesState = rootModules.readModulesManifest() + expect(modulesState?.injectedDeps?.['project-1']).toBeDefined() + expect(modulesState?.injectedDeps?.['project-1'].length).toBeGreaterThan(0) + // Injected deps should be in the global virtual store (links directory) + const injectedDepLocation = modulesState?.injectedDeps?.['project-1'][0] + expect(injectedDepLocation).toContain('links') + expect(fs.existsSync(path.join(injectedDepLocation!, 'foo.js'))).toBeTruthy() +}) diff --git a/pkg-manager/headless/src/extendProjectsWithTargetDirs.ts b/pkg-manager/headless/src/extendProjectsWithTargetDirs.ts new file mode 100644 index 0000000000..1c1775fe2d --- /dev/null +++ b/pkg-manager/headless/src/extendProjectsWithTargetDirs.ts @@ -0,0 +1,26 @@ +import { parse as parseDepPath } from '@pnpm/dependency-path' +import { type ProjectId } from '@pnpm/types' + +export function extendProjectsWithTargetDirs ( + projects: Array, + injectionTargetsByDepPath: Map +): Array { + const projectsById: Record = + Object.fromEntries(projects.map((project) => [project.id, { ...project, targetDirs: [] as string[] }])) + + for (const [depPath, locations] of injectionTargetsByDepPath) { + const parsed = parseDepPath(depPath) + if (!parsed.name || !parsed.nonSemverVersion?.startsWith('file:')) continue + const importerId = parsed.nonSemverVersion.replace(/^file:/, '') as ProjectId + if (projectsById[importerId] == null) continue + // Dedupe: only add locations that aren't already tracked + for (const location of locations) { + if (!projectsById[importerId].targetDirs.includes(location)) { + projectsById[importerId].targetDirs.push(location) + } + } + projectsById[importerId].stages = ['preinstall', 'install', 'postinstall', 'prepare', 'prepublishOnly'] + } + + return Object.values(projectsById) as Array +} diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index 42eb0b506a..a016bc7dfb 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -34,9 +34,9 @@ import { } from '@pnpm/lockfile.fs' import { writePnpFile } from '@pnpm/lockfile-to-pnp' import { - extendProjectsWithTargetDirs, nameVerFromPkgSnapshot, } from '@pnpm/lockfile.utils' +import { extendProjectsWithTargetDirs } from './extendProjectsWithTargetDirs.js' import { type LogBase, logger, @@ -86,6 +86,7 @@ import { } from '@pnpm/deps.graph-builder' import { lockfileToHoistedDepGraph } from './lockfileToHoistedDepGraph.js' import { linkDirectDeps, type LinkedDirectDep } from '@pnpm/pkg-manager.direct-dep-linker' +export { extendProjectsWithTargetDirs } from './extendProjectsWithTargetDirs.js' export type { HoistingLimits } @@ -339,7 +340,7 @@ export async function headlessInstall (opts: HeadlessOptions): Promise, + injectionTargetsByDepPath: new Map(), hoistedLocations: {} as Record, } const hierarchy = { @@ -120,9 +121,9 @@ async function _lockfileToHoistedDepGraph ( directDependenciesByImporterId, graph, hierarchy, - pkgLocationsByDepPath: fetchDepsOpts.pkgLocationsByDepPath, symlinkedDirectDependenciesByImporterId, hoistedLocations: fetchDepsOpts.hoistedLocations, + injectionTargetsByDepPath: fetchDepsOpts.injectionTargetsByDepPath, } } @@ -159,6 +160,7 @@ async function fetchDeps ( graph: DependenciesGraph lockfile: LockfileObject pkgLocationsByDepPath: Record + injectionTargetsByDepPath: Map hoistedLocations: Record } & LockfileToHoistedDepGraphOptions, modules: string, @@ -261,6 +263,15 @@ async function fetchDeps ( opts.pkgLocationsByDepPath[depPath] = [] } opts.pkgLocationsByDepPath[depPath].push(dir) + // Track directory deps for injected workspace packages + if ('directory' in pkgSnapshot.resolution && pkgSnapshot.resolution.directory != null) { + const locations = opts.injectionTargetsByDepPath.get(depPath) + if (locations) { + locations.push(dir) + } else { + opts.injectionTargetsByDepPath.set(depPath, [dir]) + } + } depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies) if (!opts.hoistedLocations[depPath]) { opts.hoistedLocations[depPath] = []