feat: store locations of deps when node-linker is set to hoisted (#5795)

Currently, when `node-linker=hoisted` is used and `node_modules` is not up-to-date, pnpm relinks all dependencies inside node_modules. This is not efficient, so with this optimisation pnpm will only relink what needs to be relinked.

.changeset/cold-pigs-burn.md

@@ -0,0 +1,8 @@
+---
+"@pnpm/fs.indexed-pkg-importer": minor
+"@pnpm/cafs-types": minor
+"@pnpm/create-cafs-store": minor
+"@pnpm/store-controller-types": minor
+---
+
+A new option added to package importer for keeping modules directory: `keepModulesDir`. When this is set to true, if a package already exist at the target location and it has a node_modules directory, then that node_modules directory is moved to the newly imported dependency. This is only needed when node-linker=hoisted is used.

.changeset/late-swans-impress.md

@@ -0,0 +1,7 @@
+---
+"pnpm": minor
+"@pnpm/core": minor
+"@pnpm/headless": minor
+---
+
+When the hoisted node linker is used, preserve `node_modules` directories when linking new dependencies. This improves performance, when installing in a project that already has a `node_modules` directory [#5795](https://github.com/pnpm/pnpm/pull/5795).

.changeset/wet-coins-greet.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/modules-yaml": minor
+---
+
+New field saved in the modules state file: `hoistedLocations`. This field maps the locations of dependencies, when `node-linker=hoisted` is used.

fs/indexed-pkg-importer/src/importIndexedDir.ts

@@ -14,11 +14,18 @@ export type ImportFile = (src: string, dest: string) => Promise<void>
 export async function importIndexedDir (
   importFile: ImportFile,
   newDir: string,
-  filenames: Record<string, string>
+  filenames: Record<string, string>,
+  opts: {
+    keepModulesDir?: boolean
+  }
 ) {
   const stage = pathTemp(path.dirname(newDir))
   try {
     await tryImportIndexedDir(importFile, stage, filenames)
+    if (opts.keepModulesDir) {
+      // Keeping node_modules is needed only when the hoisted node linker is used.
+      await moveOrMergeModulesDirs(path.join(newDir, 'node_modules'), path.join(stage, 'node_modules'))
+    }
     await renameOverwrite(stage, newDir)
   } catch (err: any) { // eslint-disable-line
     try {
@@ -37,7 +44,7 @@ export async function importIndexedDir (
         'which is an issue on case-insensitive filesystems. ' +
         `The conflicting file names are: ${JSON.stringify(conflictingFileNames)}`
       )
-      await importIndexedDir(importFile, newDir, uniqueFileMap)
+      await importIndexedDir(importFile, newDir, uniqueFileMap, opts)
       return
     }
     if (err['code'] === 'ENOENT') {
@@ -47,7 +54,7 @@ export async function importIndexedDir (
 The package linked to "${path.relative(process.cwd(), newDir)}" had \
 files with invalid names: ${invalidFilenames.join(', ')}. \
 They were renamed.`)
-      await importIndexedDir(importFile, newDir, sanitizedFilenames)
+      await importIndexedDir(importFile, newDir, sanitizedFilenames, opts)
       return
     }
     throw err
@@ -108,3 +115,29 @@ function getUniqueFileMap (fileMap: Record<string, string>) {
     uniqueFileMap,
   }
 }
+
+async function moveOrMergeModulesDirs (src: string, dest: string) {
+  try {
+    await fs.rename(src, dest)
+  } catch (err: any) { // eslint-disable-line
+    switch (err.code) {
+    case 'ENOENT':
+      // If src directory doesn't exist, there is nothing to do
+      return
+    case 'ENOTEMPTY':
+    case 'EPERM': // This error code is thrown on Windows
+      // The newly added dependency might have node_modules if it has bundled dependencies.
+      await mergeModulesDirs(src, dest)
+      return
+    default:
+      throw err
+    }
+  }
+}
+
+async function mergeModulesDirs (src: string, dest: string) {
+  const srcFiles = await fs.readdir(src)
+  const destFiles = new Set(await fs.readdir(dest))
+  const filesToMove = srcFiles.filter((file) => !destFiles.has(file))
+  await Promise.all(filesToMove.map((file) => fs.rename(path.join(src, file), path.join(dest, file))))
+}

fs/indexed-pkg-importer/src/index.ts

@@ -110,7 +110,7 @@ async function clonePkg (
   const pkgJsonPath = path.join(to, 'package.json')
 
   if (!opts.fromStore || opts.force || !await exists(pkgJsonPath)) {
-    await importIndexedDir(cloneFile, to, opts.filesMap)
+    await importIndexedDir(cloneFile, to, opts.filesMap, opts)
     return 'clone'
   }
   return undefined
@@ -130,7 +130,7 @@ async function hardlinkPkg (
     opts.force ||
     !await pkgLinkedToStore(opts.filesMap, to)
   ) {
-    await importIndexedDir(importFile, to, opts.filesMap)
+    await importIndexedDir(importFile, to, opts.filesMap, opts)
     return 'hardlink'
   }
   return undefined
@@ -188,7 +188,7 @@ export async function copyPkg (
   const pkgJsonPath = path.join(to, 'package.json')
 
   if (!opts.fromStore || opts.force || !await exists(pkgJsonPath)) {
-    await importIndexedDir(fs.copyFile, to, opts.filesMap)
+    await importIndexedDir(fs.copyFile, to, opts.filesMap, opts)
     return 'copy'
   }
   return undefined

fs/indexed-pkg-importer/test/importIndexedDir.test.ts

@@ -0,0 +1,21 @@
+import { tempDir } from '@pnpm/prepare'
+import { promises as fs, mkdirSync, writeFileSync } from 'fs'
+import path from 'path'
+import { importIndexedDir } from '../src/importIndexedDir'
+
+test('importIndexedDir() keepModulesDir merges node_modules', async () => {
+  const tmp = tempDir()
+  mkdirSync(path.join(tmp, 'src/node_modules/a'), { recursive: true })
+  writeFileSync(path.join(tmp, 'src/node_modules/a/index.js'), 'module.exports = 1')
+
+  mkdirSync(path.join(tmp, 'dest/node_modules/b'), { recursive: true })
+  writeFileSync(path.join(tmp, 'dest/node_modules/b/index.js'), 'module.exports = 1')
+
+  const newDir = path.join(tmp, 'dest')
+  const filenames = {
+    'node_modules/a/index.js': path.join(tmp, 'src/node_modules/a/index.js'),
+  }
+  await importIndexedDir(fs.link, newDir, filenames, { keepModulesDir: true })
+
+  expect(await fs.readdir(path.join(newDir, 'node_modules'))).toEqual(['a', 'b'])
+})

pkg-manager/core/src/install/index.ts

@@ -344,6 +344,7 @@ export async function mutateModules (
               nodeVersion: opts.nodeVersion,
               pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '',
             },
+            currentHoistedLocations: ctx.modulesFile?.hoistedLocations,
             patchedDependencies: patchedDependenciesWithResolvedPath,
             selectedProjectDirs: projects.map((project) => project.rootDir),
             allProjects: ctx.projects,
@@ -1177,6 +1178,7 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => {
           nodeVersion: opts.nodeVersion,
           pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '',
         },
+        currentHoistedLocations: ctx.modulesFile?.hoistedLocations,
         selectedProjectDirs: projects.map((project) => project.rootDir),
         allProjects: ctx.projects,
         prunedAt: ctx.modulesFile?.prunedAt,

pkg-manager/core/src/install/link.ts

@@ -46,9 +46,7 @@ export async function linkPackages (
   opts: {
     currentLockfile: Lockfile
     dedupeDirectDeps: boolean
-    dependenciesByProjectId: {
-      [id: string]: { [alias: string]: string }
-    }
+    dependenciesByProjectId: Record<string, Record<string, string>>
     force: boolean
     depsStateCache: DepsStateCache
     extraNodePaths: string[]
@@ -61,7 +59,7 @@ export async function linkPackages (
     linkedDependenciesByProjectId: Record<string, LinkedDependency[]>
     lockfileDir: string
     makePartialCurrentLockfile: boolean
-    outdatedDependencies: { [pkgId: string]: string }
+    outdatedDependencies: Record<string, string>
     pruneStore: boolean
     pruneVirtualStore: boolean
     registries: Registries

pkg-manager/headless/src/index.ts

@@ -120,6 +120,7 @@ export interface HeadlessOptions {
   hoistedDependencies: HoistedDependencies
   hoistPattern?: string[]
   publicHoistPattern?: string[]
+  currentHoistedLocations?: Record<string, string[]>
   lockfileDir: string
   modulesDir?: string
   virtualStoreDir?: string
@@ -283,6 +284,7 @@ export async function headlessInstall (opts: HeadlessOptions) {
     directDependenciesByImporterId,
     graph,
     hierarchy,
+    hoistedLocations,
     pkgLocationsByDepPath,
     prevGraph,
     symlinkedDirectDependenciesByImporterId,
@@ -528,6 +530,7 @@ export async function headlessInstall (opts: HeadlessOptions) {
       included: opts.include,
       injectedDeps,
       layoutVersion: LAYOUT_VERSION,
+      hoistedLocations,
       nodeLinker: opts.nodeLinker,
       packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`,
       pendingBuilds: opts.pendingBuilds,

pkg-manager/headless/src/linkHoistedModules.ts

@@ -95,36 +95,39 @@ async function linkAllPkgsInOrder (
   await Promise.all(
     Object.entries(hierarchy).map(async ([dir, deps]) => {
       const depNode = graph[dir]
-      let filesResponse!: PackageFilesResponse
-      try {
-        filesResponse = await depNode.fetchingFiles()
-      } catch (err: any) { // eslint-disable-line
-        if (depNode.optional) return
-        throw err
-      }
+      if (depNode.fetchingFiles) {
+        let filesResponse!: PackageFilesResponse
+        try {
+          filesResponse = await depNode.fetchingFiles()
+        } catch (err: any) { // eslint-disable-line
+          if (depNode.optional) return
+          throw err
+        }
 
-      let sideEffectsCacheKey: string | undefined
-      if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) {
-        sideEffectsCacheKey = _calcDepState(dir, {
-          isBuilt: !opts.ignoreScripts && depNode.requiresBuild,
-          patchFileHash: depNode.patchFile?.hash,
+        let sideEffectsCacheKey: string | undefined
+        if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) {
+          sideEffectsCacheKey = _calcDepState(dir, {
+            isBuilt: !opts.ignoreScripts && depNode.requiresBuild,
+            patchFileHash: depNode.patchFile?.hash,
+          })
+        }
+        const { importMethod, isBuilt } = await storeController.importPackage(depNode.dir, {
+          filesResponse,
+          force: opts.force || depNode.depPath !== prevGraph[dir]?.depPath,
+          keepModulesDir: true,
+          requiresBuild: depNode.requiresBuild || depNode.patchFile != null,
+          sideEffectsCacheKey,
         })
+        if (importMethod) {
+          progressLogger.debug({
+            method: importMethod,
+            requester: opts.lockfileDir,
+            status: 'imported',
+            to: depNode.dir,
+          })
+        }
+        depNode.isBuilt = isBuilt
       }
-      const { importMethod, isBuilt } = await storeController.importPackage(depNode.dir, {
-        filesResponse,
-        force: opts.force || depNode.depPath !== prevGraph[dir]?.depPath,
-        requiresBuild: depNode.requiresBuild || depNode.patchFile != null,
-        sideEffectsCacheKey,
-      })
-      if (importMethod) {
-        progressLogger.debug({
-          method: importMethod,
-          requester: opts.lockfileDir,
-          status: 'imported',
-          to: depNode.dir,
-        })
-      }
-      depNode.isBuilt = isBuilt
       return linkAllPkgsInOrder(storeController, graph, prevGraph, deps, dir, opts)
     })
   )

pkg-manager/headless/src/lockfileToDepGraph.ts

@@ -80,6 +80,7 @@ export interface LockfileToDepGraphResult {
   directDependenciesByImporterId: DirectDependenciesByImporterId
   graph: DependenciesGraph
   hierarchy?: DepHierarchy
+  hoistedLocations?: Record<string, string[]>
   symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId
   prevGraph?: DependenciesGraph
   pkgLocationsByDepPath?: Record<string, string[]>

pkg-manager/headless/src/lockfileToHoistedDepGraph.ts

@@ -32,6 +32,7 @@ export interface LockfileToHoistedDepGraphOptions {
   externalDependencies?: Set<string>
   importerIds: string[]
   include: IncludedDependencies
+  currentHoistedLocations?: Record<string, string[]>
   lockfileDir: string
   nodeVersion: string
   pnpmVersion: string
@@ -73,6 +74,7 @@ async function _lockfileToHoistedDepGraph (
     lockfile,
     graph,
     pkgLocationsByDepPath: {},
+    hoistedLocations: {} as Record<string, string[]>,
   }
   const hierarchy = {
     [opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies),
@@ -102,6 +104,7 @@ async function _lockfileToHoistedDepGraph (
     hierarchy,
     pkgLocationsByDepPath: fetchDepsOpts.pkgLocationsByDepPath,
     symlinkedDirectDependenciesByImporterId,
+    hoistedLocations: fetchDepsOpts.hoistedLocations,
   }
 }
 
@@ -136,6 +139,7 @@ async function fetchDeps (
     graph: DependenciesGraph
     lockfile: Lockfile
     pkgLocationsByDepPath: Record<string, string[]>
+    hoistedLocations: Record<string, string[]>
   } & LockfileToHoistedDepGraphOptions,
   modules: string,
   deps: Set<HoisterResult>
@@ -173,25 +177,31 @@ async function fetchDeps (
       return
     }
     const dir = path.join(modules, dep.name)
+    const depLocation = path.relative(opts.lockfileDir, dir)
     const resolution = pkgSnapshotToResolution(depPath, pkgSnapshot, opts.registries)
     let fetchResponse!: ReturnType<FetchPackageToStoreFunction>
-    try {
-      fetchResponse = opts.storeController.fetchPackage({
-        force: false,
-        lockfileDir: opts.lockfileDir,
-        pkg: {
-          id: packageId,
-          resolution,
-        },
-        expectedPkg: {
-          name: pkgName,
-          version: pkgVersion,
-        },
-      })
-      if (fetchResponse instanceof Promise) fetchResponse = await fetchResponse
-    } catch (err: any) { // eslint-disable-line
-      if (pkgSnapshot.optional) return
-      throw err
+    const skipFetch = opts.currentHoistedLocations?.[depPath]?.includes(depLocation)
+    if (skipFetch) {
+      fetchResponse = {} as any // eslint-disable-line @typescript-eslint/no-explicit-any
+    } else {
+      try {
+        fetchResponse = opts.storeController.fetchPackage({
+          force: false,
+          lockfileDir: opts.lockfileDir,
+          pkg: {
+            id: packageId,
+            resolution,
+          },
+          expectedPkg: {
+            name: pkgName,
+            version: pkgVersion,
+          },
+        })
+        if (fetchResponse instanceof Promise) fetchResponse = await fetchResponse
+      } catch (err: any) { // eslint-disable-line
+        if (pkgSnapshot.optional) return
+        throw err
+      }
     }
     opts.graph[dir] = {
       alias: dep.name,
@@ -216,6 +226,10 @@ async function fetchDeps (
     }
     opts.pkgLocationsByDepPath[depPath].push(dir)
     depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies)
+    if (!opts.hoistedLocations[depPath]) {
+      opts.hoistedLocations[depPath] = []
+    }
+    opts.hoistedLocations[depPath].push(depLocation)
     opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationsByDepPath, opts)
   }))
   return depHierarchy

pkg-manager/modules-yaml/src/index.ts

@@ -31,6 +31,7 @@ export interface Modules {
   storeDir: string
   virtualStoreDir: string
   injectedDeps?: Record<string, string[]>
+  hoistedLocations?: Record<string, string[]>
 }
 
 export async function readModulesManifest (modulesDir: string): Promise<Modules | null> {

store/cafs-types/src/index.ts

@@ -30,6 +30,7 @@ export interface ImportPackageOpts {
   sideEffectsCacheKey?: string
   filesResponse: PackageFilesResponse
   force: boolean
+  keepModulesDir?: boolean
 }
 
 export type ImportPackageFunction = (

store/create-cafs-store/src/index.ts

@@ -33,7 +33,12 @@ function createPackageImporter (
       ? 'clone-or-copy'
       : (opts.filesResponse.packageImportMethod ?? packageImportMethod)
     const impPkg = cachedImporterCreator(pkgImportMethod)
-    const importMethod = await impPkg(to, { filesMap, fromStore: opts.filesResponse.fromStore, force: opts.force })
+    const importMethod = await impPkg(to, {
+      filesMap,
+      fromStore: opts.filesResponse.fromStore,
+      force: opts.force,
+      keepModulesDir: Boolean(opts.keepModulesDir),
+    })
     return { importMethod, isBuilt }
   }
 }

store/store-controller-types/src/index.ts

@@ -147,6 +147,7 @@ export interface ImportOptions {
   filesMap: FilesMap
   force: boolean
   fromStore: boolean
+  keepModulesDir?: boolean
 }
 
 export type ImportIndexedPackage = (to: string, opts: ImportOptions) => Promise<string | undefined>