perf: faster linking of commands of hoisted packages (#7212)

.changeset/strange-crabs-marry.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/link-bins": patch
+"@pnpm/hoist": patch
+"pnpm": patch
+---
+
+Improved the performance of linking bins of hoisted dependencies to `node_modules/.pnpm/node_modules/.bin` [#7212](https://github.com/pnpm/pnpm/pull/7212).

pkg-manager/hoist/src/index.ts

@@ -2,7 +2,7 @@ import fs from 'fs'
 import path from 'path'
 import { linkLogger } from '@pnpm/core-loggers'
 import { WANTED_LOCKFILE } from '@pnpm/constants'
-import { linkBins, type WarnFunction } from '@pnpm/link-bins'
+import { linkBinsOfPkgsByAliases, type WarnFunction } from '@pnpm/link-bins'
 import {
   type Lockfile,
   nameVerFromPkgSnapshot,
@@ -56,8 +56,9 @@ export async function hoist (
 
   const getAliasHoistType = createGetAliasHoistType(opts.publicHoistPattern, opts.privateHoistPattern)
 
-  const hoistedDependencies = await hoistGraph(deps, opts.lockfile.importers['.']?.specifiers ?? {}, {
+  const { hoistedDependencies, hoistedAliasesWithBins } = await hoistGraph(deps, opts.lockfile.importers['.']?.specifiers ?? {}, {
     getAliasHoistType,
+    lockfile: opts.lockfile,
   })
 
   await symlinkHoistedDependencies(hoistedDependencies, {
@@ -74,6 +75,7 @@ export async function hoist (
   // are in the same directory as the regular dependencies.
   await linkAllBins(opts.privateHoistedModulesDir, {
     extraNodePaths: opts.extraNodePath,
+    hoistedAliasesWithBins,
     preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
   })
 
@@ -97,6 +99,7 @@ function createGetAliasHoistType (
 
 interface LinkAllBinsOptions {
   extraNodePaths?: string[]
+  hoistedAliasesWithBins: string[]
   preferSymlinkedExecutables?: boolean
 }
 
@@ -107,9 +110,10 @@ async function linkAllBins (modulesDir: string, opts: LinkAllBinsOptions) {
     logger.info({ message, prefix: path.join(modulesDir, '../..') })
   }
   try {
-    await linkBins(modulesDir, bin, {
+    await linkBinsOfPkgsByAliases(opts.hoistedAliasesWithBins, bin, {
       allowExoticManifests: true,
       extraNodePaths: opts.extraNodePaths,
+      modulesDir,
       preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
       warn,
     })
@@ -160,15 +164,22 @@ export interface Dependency {
   depth: number
 }
 
+interface HoistGraphResult {
+  hoistedDependencies: HoistedDependencies
+  hoistedAliasesWithBins: string[]
+}
+
 async function hoistGraph (
   depNodes: Dependency[],
   currentSpecifiers: Record<string, string>,
   opts: {
     getAliasHoistType: GetAliasHoistType
+    lockfile: Lockfile
   }
-): Promise<HoistedDependencies> {
+): Promise<HoistGraphResult> {
   const hoistedAliases = new Set(Object.keys(currentSpecifiers))
   const hoistedDependencies: HoistedDependencies = {}
+  const hoistedAliasesWithBins = new Set<string>()
 
   depNodes
     // sort by depth and then alphabetically
@@ -186,6 +197,9 @@ async function hoistGraph (
         if (hoistedAliases.has(childAliasNormalized)) {
           continue
         }
+        if (opts.lockfile.packages?.[childPath]?.hasBin) {
+          hoistedAliasesWithBins.add(childAlias)
+        }
         hoistedAliases.add(childAliasNormalized)
         if (!hoistedDependencies[childPath]) {
           hoistedDependencies[childPath] = {}
@@ -194,7 +208,7 @@ async function hoistGraph (
       }
     })
 
-  return hoistedDependencies
+  return { hoistedDependencies, hoistedAliasesWithBins: Array.from(hoistedAliasesWithBins) }
 }
 
 async function symlinkHoistedDependencies (

pkg-manager/link-bins/src/index.ts

@@ -44,6 +44,23 @@ export async function linkBins (
   const allDeps = await readModulesDir(modulesDir)
   // If the modules dir does not exist, do nothing
   if (allDeps === null) return []
+  return linkBinsOfPkgsByAliases(allDeps, binsDir, {
+    ...opts,
+    modulesDir,
+  })
+}
+
+export async function linkBinsOfPkgsByAliases (
+  depsAliases: string[],
+  binsDir: string,
+  opts: LinkBinOptions & {
+    modulesDir: string
+    allowExoticManifests?: boolean
+    nodeExecPathByAlias?: Record<string, string>
+    projectManifest?: ProjectManifest
+    warn: WarnFunction
+  }
+): Promise<string[]> {
   const pkgBinOpts = {
     allowExoticManifests: false,
     ...opts,
@@ -53,9 +70,9 @@ export async function linkBins (
     : new Set(Object.keys(getAllDependenciesFromManifest(opts.projectManifest)))
   const allCmds = unnest(
     (await Promise.all(
-      allDeps
+      depsAliases
         .map((alias) => ({
-          depDir: path.resolve(modulesDir, alias),
+          depDir: path.resolve(opts.modulesDir, alias),
           isDirectDependency: directDependencies?.has(alias),
           nodeExecPath: opts.nodeExecPathByAlias?.[alias],
         }))

pkg-manager/link-bins/test/index.ts

@@ -1,10 +1,11 @@
 /// <reference path="../../../__typings__/index.d.ts"/>
-import { promises as fs, writeFileSync } from 'fs'
+import { promises as fs, writeFileSync, readdirSync, existsSync, readFileSync } from 'fs'
 import path from 'path'
 import { logger, globalWarn } from '@pnpm/logger'
 import {
   linkBins,
   linkBinsOfPackages,
+  linkBinsOfPkgsByAliases,
 } from '@pnpm/link-bins'
 import { fixtures } from '@pnpm/test-fixtures'
 import CMD_EXTENSION from 'cmd-extension'
@@ -167,6 +168,36 @@ test('linkBinsOfPackages()', async () => {
   expect(content).toMatch('node_modules/simple/index.js')
 })
 
+test('linkBinsOfPkgsByAliases()', async () => {
+  const binTarget = tempy.directory()
+  const simpleFixture = f.prepare('simple-fixture')
+
+  await linkBinsOfPkgsByAliases(
+    [],
+    binTarget,
+    {
+      modulesDir: path.join(simpleFixture, 'node_modules'),
+      warn: () => {},
+    }
+  )
+  expect(readdirSync(binTarget)).toEqual([])
+
+  await linkBinsOfPkgsByAliases(
+    ['simple'],
+    binTarget,
+    {
+      modulesDir: path.join(simpleFixture, 'node_modules'),
+      warn: () => {},
+    }
+  )
+
+  expect(readdirSync(binTarget)).toEqual(getExpectedBins(['simple']))
+  const binLocation = path.join(binTarget, 'simple')
+  expect(existsSync(binLocation)).toBe(true)
+  const content = readFileSync(binLocation, 'utf8')
+  expect(content).toMatch('node_modules/simple/index.js')
+})
+
 test('linkBins() resolves conflicts. Prefer packages that use their name as bin name', async () => {
   const binTarget = tempy.directory()
   const binNameConflictsFixture = f.prepare('bin-name-conflicts')