perf: don't prune the modules directory on each install (#3124)

ref #3115

.changeset/early-foxes-behave.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/config": minor
+---
+
+New setting added: `modules-cache-max-age`. The default value of the setting is 10080 (7 days in seconds). `modules-cache-max-age` is the time in minutes after which pnpm should remove the orphan packages from node_modules.

.changeset/fresh-eels-collect.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/headless": minor
+---
+
+New option added: `prunedAt`. `prunedAt` is the stringified UTC time of the last time the node_modules was cleared from orphan packages.

.changeset/heavy-balloons-change.md

@@ -0,0 +1,5 @@
+---
+"supi": minor
+---
+
+New option added: `modulesCacheMaxAge`. The default value of the setting is 10080 (7 days in seconds). `modulesCacheMaxAge` is the time in minutes after which pnpm should remove the orphan packages from node_modules.

.changeset/polite-trees-invent.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/plugin-commands-rebuild": patch
+---
+
+`prunedAt` is set for the modules meta file.

.changeset/small-coins-beam.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/modules-yaml": major
+---
+
+New required option added to the modules meta object: `prunedAt`. `prunedAt` is the stringified UTC date when the virtual store was last cleared.

.changeset/smooth-dolphins-think.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/modules-cleaner": minor
+---
+
+`prune()` accepts a new option: `pruneVirtualStore`. When `pruneVirtualStore` is `true`, any unreferenced packages are removed from the virtual store (from `node_modules/.pnpm`).

.changeset/wet-lobsters-kiss.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/headless": minor
+---
+
+New option added: `pruneVirtualStore`. When `true`, orphan packages should be removed from the virtual store.

packages/config/src/Config.ts

@@ -119,6 +119,7 @@ export interface Config {
   symlink: boolean
   enablePnp?: boolean
   enableModulesDir: boolean
+  modulesCacheMaxAge: number
 
   registries: Registries
   ignoreWorkspaceRootCheck: boolean

packages/config/src/index.ts

@@ -51,6 +51,7 @@ export const types = Object.assign({
   'lockfile-directory': String, // TODO: deprecate
   'lockfile-only': Boolean,
   loglevel: ['silent', 'error', 'warn', 'info', 'debug'],
+  'modules-cache-max-age': Number,
   'modules-dir': String,
   'network-concurrency': Number,
   'node-linker': ['pnp'],
@@ -157,6 +158,7 @@ export default async (
     'hoist-pattern': ['*'],
     'ignore-workspace-root-check': false,
     'link-workspace-packages': true,
+    'modules-cache-max-age': 7 * 24 * 60, // 7 days
     'package-lock': npmDefaults['package-lock'],
     pending: false,
     'prefer-workspace-packages': false,

packages/headless/src/index.ts

@@ -89,6 +89,7 @@ export interface HeadlessOptions {
     pruneDirectDependencies?: boolean
     rootDir: string
   }>
+  prunedAt?: string
   hoistedDependencies: HoistedDependencies
   hoistPattern?: string[]
   publicHoistPattern?: string[]
@@ -113,6 +114,7 @@ export interface HeadlessOptions {
     version: string
   }
   pruneStore: boolean
+  pruneVirtualStore?: boolean
   wantedLockfile?: Lockfile
   ownLifecycleHooksStdio?: 'inherit' | 'pipe'
   pendingBuilds: string[]
@@ -178,6 +180,7 @@ export default async (opts: HeadlessOptions) => {
         include: opts.include,
         lockfileDir,
         pruneStore: opts.pruneStore,
+        pruneVirtualStore: opts.pruneVirtualStore,
         publicHoistedModulesDir: (opts.publicHoistPattern && publicHoistedModulesDir) ?? undefined,
         registries: opts.registries,
         skipped,
@@ -400,6 +403,8 @@ export default async (opts: HeadlessOptions) => {
       packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`,
       pendingBuilds: opts.pendingBuilds,
       publicHoistPattern: opts.publicHoistPattern,
+      prunedAt: opts.pruneVirtualStore === true || opts.prunedAt == null
+        ? new Date().toUTCString() : opts.prunedAt,
       registries: opts.registries,
       skipped: Array.from(skipped),
       storeDir: opts.storeDir,

packages/modules-cleaner/package.json

@@ -37,10 +37,12 @@
     "@pnpm/types": "workspace:6.4.0",
     "@zkochan/rimraf": "^1.0.0",
     "dependency-path": "workspace:5.1.1",
+    "mz": "^2.7.0",
     "ramda": "^0.27.1"
   },
   "devDependencies": {
     "@pnpm/logger": "^3.2.3",
+    "@types/mz": "^2.7.3",
     "@types/ramda": "^0.27.35"
   },
   "bugs": {

packages/modules-cleaner/src/prune.ts

@@ -20,6 +20,7 @@ import {
 } from '@pnpm/types'
 import { depPathToFilename } from 'dependency-path'
 import removeDirectDependency from './removeDirectDependency'
+import fs = require('mz/fs')
 import path = require('path')
 import rimraf = require('@zkochan/rimraf')
 import R = require('ramda')
@@ -42,6 +43,7 @@ export default async function prune (
     wantedLockfile: Lockfile
     currentLockfile: Lockfile
     pruneStore?: boolean
+    pruneVirtualStore?: boolean
     registries: Registries
     skipped: Set<string>
     virtualStoreDir: string
@@ -114,8 +116,9 @@ export default async function prune (
     removed: orphanPkgIds.size,
   })
 
-  if (!opts.dryRun && orphanDepPaths.length) {
+  if (!opts.dryRun) {
     if (
+      orphanDepPaths.length &&
       opts.currentLockfile.packages &&
       (opts.hoistedModulesDir != null || opts.publicHoistedModulesDir != null)
     ) {
@@ -140,24 +143,59 @@ export default async function prune (
       }))
     }
 
-    await Promise.all(orphanDepPaths.map(async (orphanDepPath) => {
-      const pathToRemove = path.join(opts.virtualStoreDir, depPathToFilename(orphanDepPath, opts.lockfileDir))
-      removalLogger.debug(pathToRemove)
-      try {
-        await rimraf(pathToRemove)
-      } catch (err) {
-        logger.warn({
-          error: err,
-          message: `Failed to remove "${pathToRemove}"`,
-          prefix: opts.lockfileDir,
-        })
+    if (opts.pruneVirtualStore !== false) {
+      const _tryRemovePkg = tryRemovePkg.bind(null, opts.lockfileDir, opts.virtualStoreDir)
+      await Promise.all(
+        orphanDepPaths
+          .map((orphanDepPath) => depPathToFilename(orphanDepPath, opts.lockfileDir))
+          .map((orphanDepPath) => _tryRemovePkg(orphanDepPath))
+      )
+      const neededPkgs: Set<string> = new Set()
+      for (const depPath of Object.keys(opts.wantedLockfile.packages ?? {})) {
+        if (opts.skipped.has(depPath)) continue
+        neededPkgs.add(depPathToFilename(depPath, opts.lockfileDir))
       }
-    }))
+      const availablePkgs = await readVirtualStoreDir(opts.virtualStoreDir, opts.lockfileDir)
+      await Promise.all(
+        availablePkgs
+          .filter((availablePkg) => !neededPkgs.has(availablePkg))
+          .map((orphanDepPath) => _tryRemovePkg(orphanDepPath))
+      )
+    }
   }
 
   return new Set(orphanDepPaths)
 }
 
+async function readVirtualStoreDir (virtualStoreDir: string, lockfileDir: string) {
+  try {
+    return await fs.readdir(virtualStoreDir)
+  } catch (err) {
+    if (err.code !== 'ENOENT') {
+      logger.warn({
+        error: err,
+        message: `Failed to read virtualStoreDir at "${virtualStoreDir}"`,
+        prefix: lockfileDir,
+      })
+    }
+    return []
+  }
+}
+
+async function tryRemovePkg (lockfileDir: string, virtualStoreDir: string, pkgDir: string) {
+  const pathToRemove = path.join(virtualStoreDir, pkgDir)
+  removalLogger.debug(pathToRemove)
+  try {
+    await rimraf(pathToRemove)
+  } catch (err) {
+    logger.warn({
+      error: err,
+      message: `Failed to remove "${pathToRemove}"`,
+      prefix: lockfileDir,
+    })
+  }
+}
+
 function mergeDependencies (projectSnapshot: ProjectSnapshot): { [depName: string]: string } {
   return R.mergeAll(
     DEPENDENCIES_FIELDS.map((depType) => projectSnapshot[depType] ?? {})

packages/modules-yaml/src/index.ts

@@ -20,6 +20,7 @@ export interface Modules {
   layoutVersion: number
   packageManager: string
   pendingBuilds: string[]
+  prunedAt: string
   registries?: Registries // nullable for backward compatibility
   shamefullyHoist?: boolean // for backward compatibility
   publicHoistPattern?: string[]
@@ -74,6 +75,9 @@ export async function read (modulesDir: string): Promise<Modules | null> {
     }
     break
   }
+  if (!modules.prunedAt) {
+    modules.prunedAt = new Date().toUTCString()
+  }
   return modules
 }
 

packages/modules-yaml/test/index.ts

@@ -18,6 +18,7 @@ test('write() and read()', async () => {
     packageManager: 'pnpm@2',
     pendingBuilds: [],
     publicHoistPattern: [],
+    prunedAt: new Date().toUTCString(),
     registries: {
       default: 'https://registry.npmjs.org/',
     },

packages/plugin-commands-rebuild/src/implementation/index.ts

@@ -171,6 +171,7 @@ export async function rebuild (
   }
 
   await writeModulesYaml(ctx.rootModulesDir, {
+    prunedAt: new Date().toUTCString(),
     ...ctx.modulesFile,
     hoistedDependencies: ctx.hoistedDependencies,
     hoistPattern: ctx.hoistPattern,

packages/plugin-commands-store/test/storePrune.ts

@@ -20,7 +20,7 @@ test('remove unreferenced packages', async () => {
   const storeDir = path.resolve('store')
 
   await execa('node', [pnpmBin, 'add', 'is-negative@2.1.0', '--store-dir', storeDir, '--registry', REGISTRY])
-  await execa('node', [pnpmBin, 'remove', 'is-negative', '--store-dir', storeDir], { env: { npm_config_registry: REGISTRY } })
+  await execa('node', [pnpmBin, 'remove', 'is-negative', '--store-dir', storeDir, '--config.modules-cache-max-age=0'], { env: { npm_config_registry: REGISTRY } })
 
   await project.storeHas('is-negative', '2.1.0')
 

packages/supi/src/install/extendInstallOptions.ts

@@ -69,6 +69,7 @@ export interface StrictInstallOptions {
   dir: string
   symlink: boolean
   enableModulesDir: boolean
+  modulesCacheMaxAge: number
 
   hoistPattern: string[] | undefined
   forceHoistPattern: boolean
@@ -144,6 +145,7 @@ const defaults = async (opts: InstallOptions) => {
     verifyStoreIntegrity: true,
     workspacePackages: {},
     enableModulesDir: true,
+    modulesCacheMaxAge: 7 * 24 * 60,
   } as StrictInstallOptions
 }
 

packages/supi/src/install/index.ts

@@ -138,6 +138,9 @@ export async function mutateModules (
   opts['forceNewModules'] = installsOnly
   opts['autofixMergeConflicts'] = !opts.frozenLockfile
   const ctx = await getContext(projects, opts)
+  const pruneVirtualStore = ctx.modulesFile?.prunedAt && opts.modulesCacheMaxAge > 0
+    ? cacheExpired(ctx.modulesFile.prunedAt, opts.modulesCacheMaxAge)
+    : true
   const rootProjectManifest = ctx.projects.find(({ id }) => id === '.')?.manifest ??
     // When running install/update on a subset of projects, the root project might not be included,
     // so reading its manifest explicitly hear.
@@ -240,6 +243,8 @@ export async function mutateModules (
               pruneDirectDependencies?: boolean
             }>,
             pruneStore: opts.pruneStore,
+            prunedAt: ctx.modulesFile?.prunedAt,
+            pruneVirtualStore,
             publicHoistPattern: ctx.publicHoistPattern,
             rawConfig: opts.rawConfig,
             registries: opts.registries,
@@ -439,6 +444,7 @@ export async function mutateModules (
       needsFullResolution,
       neverBuiltDependencies,
       overrides,
+      pruneVirtualStore,
       update: opts.update || !installsOnly,
       updateLockfileMinorVersion: true,
     })
@@ -458,6 +464,10 @@ export async function mutateModules (
   }
 }
 
+function cacheExpired (prunedAt: string, maxAgeInMinutes: number) {
+  return ((Date.now() - new Date(prunedAt).valueOf()) / (1000 * 60)) > maxAgeInMinutes
+}
+
 async function isExternalLink (storeDir: string, modules: string, pkgName: string) {
   const link = await isInnerLink(modules, pkgName)
 
@@ -588,6 +598,7 @@ async function installInContext (
     overrides?: Record<string, string>
     updateLockfileMinorVersion: boolean
     preferredVersions?: PreferredVersions
+    pruneVirtualStore: boolean
     currentLockfileIsUpToDate: boolean
   }
 ) {
@@ -720,6 +731,7 @@ async function installInContext (
         makePartialCurrentLockfile: opts.makePartialCurrentLockfile,
         outdatedDependencies,
         pruneStore: opts.pruneStore,
+        pruneVirtualStore: opts.pruneVirtualStore,
         publicHoistPattern: ctx.publicHoistPattern,
         registries: ctx.registries,
         rootModulesDir: ctx.rootModulesDir,
@@ -857,6 +869,9 @@ async function installInContext (
           packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`,
           pendingBuilds: ctx.pendingBuilds,
           publicHoistPattern: ctx.publicHoistPattern,
+          prunedAt: opts.pruneVirtualStore || ctx.modulesFile == null
+            ? new Date().toUTCString()
+            : ctx.modulesFile.prunedAt,
           registries: ctx.registries,
           skipped: Array.from(ctx.skipped),
           storeDir: ctx.storeDir,

packages/supi/src/install/link.ts

@@ -51,6 +51,7 @@ export default async function linkPackages (
     makePartialCurrentLockfile: boolean
     outdatedDependencies: {[pkgId: string]: string}
     pruneStore: boolean
+    pruneVirtualStore: boolean
     registries: Registries
     rootModulesDir: string
     sideEffectsCacheRead: boolean
@@ -97,6 +98,7 @@ export default async function linkPackages (
     include: opts.include,
     lockfileDir: opts.lockfileDir,
     pruneStore: opts.pruneStore,
+    pruneVirtualStore: opts.pruneVirtualStore,
     publicHoistedModulesDir: (opts.publicHoistPattern && opts.rootModulesDir) ?? undefined,
     registries: opts.registries,
     skipped: opts.skipped,

packages/supi/test/install/modulesCache.ts

@@ -0,0 +1,84 @@
+import { write as writeModulesYaml } from '@pnpm/modules-yaml'
+import { prepareEmpty } from '@pnpm/prepare'
+import {
+  addDependenciesToPackage,
+  install,
+  mutateModules,
+} from 'supi'
+import { testDefaults } from '../utils'
+import path = require('path')
+
+test('the modules cache is pruned when it expires', async () => {
+  const project = prepareEmpty()
+
+  let manifest = await install({
+    dependencies: {
+      'is-positive': '1.0.0',
+      'is-negative': '1.0.0',
+    },
+  }, await testDefaults())
+
+  const modulesFile = await project.readModulesManifest()
+
+  expect(modulesFile?.prunedAt).toBeTruthy()
+
+  manifest = (await mutateModules([
+    {
+      dependencyNames: ['is-negative'],
+      manifest,
+      mutation: 'uninstallSome',
+      rootDir: process.cwd(),
+    },
+  ], await testDefaults({})))[0].manifest
+
+  await project.has('.pnpm/is-negative@1.0.0/node_modules/is-negative')
+
+  const prunedAt = new Date()
+  prunedAt.setMinutes(prunedAt.getMinutes() - 3)
+  modulesFile!.prunedAt = prunedAt.toString()
+  await writeModulesYaml(path.resolve('node_modules'), modulesFile as any) // eslint-disable-line
+
+  await addDependenciesToPackage(manifest,
+    ['is-negative@2.0.0'],
+    await testDefaults({ modulesCacheMaxAge: 2 })
+  )
+
+  await project.hasNot('.pnpm/is-negative@1.0.0/node_modules/is-negative')
+})
+
+test('the modules cache is pruned when it expires and headless install is used', async () => {
+  const project = prepareEmpty()
+
+  let manifest = await install({
+    dependencies: {
+      'is-positive': '1.0.0',
+      'is-negative': '1.0.0',
+    },
+  }, await testDefaults())
+
+  const modulesFile = await project.readModulesManifest()
+
+  expect(modulesFile?.prunedAt).toBeTruthy()
+
+  manifest = (await mutateModules([
+    {
+      dependencyNames: ['is-negative'],
+      manifest,
+      mutation: 'uninstallSome',
+      rootDir: process.cwd(),
+    },
+  ], await testDefaults({ lockfileOnly: true })))[0].manifest
+
+  manifest = await install(manifest, await testDefaults({ frozenLockfile: true }))
+
+  await project.has('.pnpm/is-negative@1.0.0/node_modules/is-negative')
+
+  const prunedAt = new Date()
+  prunedAt.setMinutes(prunedAt.getMinutes() - 3)
+  modulesFile!.prunedAt = prunedAt.toString()
+  await writeModulesYaml(path.resolve('node_modules'), modulesFile as any) // eslint-disable-line
+
+  await install(manifest, await testDefaults({ frozenLockfile: true, modulesCacheMaxAge: 2 }))
+
+  await project.hasNot('.pnpm/is-negative@1.0.0/node_modules/is-negative')
+})

packages/supi/test/install/multipleImporters.ts

@@ -247,6 +247,7 @@ test('dependencies of other importers are not pruned when installing for a subse
   await addDependenciesToPackage(manifest, ['is-positive@2'], await testDefaults({
     dir: path.resolve('project-1'),
     lockfileDir: process.cwd(),
+    modulesCacheMaxAge: 0,
   }))
 
   await projects['project-1'].has('is-positive')
@@ -312,7 +313,10 @@ test('dependencies of other importers are not pruned when (headless) installing
     lockfileDir: process.cwd(),
     lockfileOnly: true,
   }))
-  await mutateModules(importers.slice(0, 1), await testDefaults({ frozenLockfile: true }))
+  await mutateModules(importers.slice(0, 1), await testDefaults({
+    frozenLockfile: true,
+    modulesCacheMaxAge: 0,
+  }))
 
   await projects['project-1'].has('is-positive')
   await projects['project-2'].has('is-negative')
@@ -963,7 +967,10 @@ test('remove dependencies of a project that was removed from the workspace (duri
     await project.has('.pnpm/is-negative@1.0.0')
   }
 
-  await mutateModules(importers.slice(0, 1), await testDefaults({ preferFrozenLockfile: false }))
+  await mutateModules(importers.slice(0, 1), await testDefaults({
+    preferFrozenLockfile: false,
+    modulesCacheMaxAge: 0,
+  }))
   {
     const currentLockfile = await project.readCurrentLockfile()
     expect(Object.keys(currentLockfile.importers)).toStrictEqual(['project-1'])

packages/supi/test/install/peerDependencies.ts

@@ -51,7 +51,9 @@ test('peer dependency is grouped with dependency when peer is resolved not from
 // Covers https://github.com/pnpm/pnpm/issues/1133
 test('nothing is needlessly removed from node_modules', async () => {
   prepareEmpty()
-  const opts = await testDefaults()
+  const opts = await testDefaults({
+    modulesCacheMaxAge: 0,
+  })
   const manifest = await addDependenciesToPackage({}, ['using-ajv', 'ajv-keywords@1.5.0'], opts)
 
   expect(await exists(path.resolve('node_modules/.pnpm/ajv-keywords@1.5.0_ajv@4.10.4/node_modules/ajv'))).toBeTruthy()
@@ -253,7 +255,7 @@ test('top peer dependency is linked on subsequent install. Reverse order', async
 
   const manifest = await addDependenciesToPackage({}, ['abc-parent-with-ab@1.0.0'], await testDefaults())
 
-  await addDependenciesToPackage(manifest, ['peer-c@1.0.0'], await testDefaults())
+  await addDependenciesToPackage(manifest, ['peer-c@1.0.0'], await testDefaults({ modulesCacheMaxAge: 0 }))
 
   expect(await exists(path.resolve('node_modules/.pnpm/abc-parent-with-ab@1.0.0/node_modules/abc-parent-with-ab'))).toBeFalsy()
   expect(await exists(path.resolve('node_modules/.pnpm/abc-parent-with-ab@1.0.0_peer-c@1.0.0/node_modules/abc-parent-with-ab'))).toBeTruthy()

pnpm-lock.yaml

@@ -1183,10 +1183,12 @@ importers:
       '@pnpm/types': link:../types
       '@zkochan/rimraf': 1.0.0
       dependency-path: link:../dependency-path
+      mz: 2.7.0
       ramda: 0.27.1
     devDependencies:
       '@pnpm/logger': 3.2.3
       '@pnpm/modules-cleaner': 'link:'
+      '@types/mz': 2.7.3
       '@types/ramda': 0.27.36
     specifiers:
       '@pnpm/core-loggers': workspace:5.0.3
@@ -1199,9 +1201,11 @@ importers:
       '@pnpm/remove-bins': workspace:1.0.11
       '@pnpm/store-controller-types': workspace:9.2.1
       '@pnpm/types': workspace:6.4.0
+      '@types/mz': ^2.7.3
       '@types/ramda': ^0.27.35
       '@zkochan/rimraf': ^1.0.0
       dependency-path: workspace:5.1.1
+      mz: ^2.7.0
       ramda: ^0.27.1
   packages/modules-yaml:
     dependencies: