feat: merge readPackage hook from opts and pnpmfile

Prevent hook provided in opts from being discarded

Closes pnpm#5306

.changeset/rich-grapes-jog.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/core": patch
+"@pnpm/plugin-commands-installation": patch
+---
+
+Combining readPackage hook from options and from pnpmfile

packages/core/src/install/index.ts

@@ -51,6 +51,7 @@ import {
   DependenciesField,
   DependencyManifest,
   PackageExtension,
+  PackageManifest,
   PeerDependencyIssues,
   PeerDependencyRules,
   ProjectManifest,
@@ -152,6 +153,9 @@ export type MutatedProject = ProjectOptions & DependenciesMutation
 
 export type MutateModulesOptions = InstallOptions & {
   preferredVersions?: PreferredVersions
+  hooks?: {
+    readPackage?: ReadPackageHook[] | ReadPackageHook
+  } | InstallOptions['hooks']
 }
 
 export async function mutateModules (
@@ -585,7 +589,7 @@ export function createReadPackageHook (
     overrides?: Record<string, string>
     packageExtensions?: Record<string, PackageExtension>
     peerDependencyRules?: PeerDependencyRules
-    readPackageHook?: ReadPackageHook
+    readPackageHook?: ReadPackageHook[] | ReadPackageHook
   }
 ): ReadPackageHook | undefined {
   const hooks: ReadPackageHook[] = []
@@ -608,15 +612,18 @@ export function createReadPackageHook (
   ) {
     hooks.push(createPeerDependencyPatcher(peerDependencyRules))
   }
+  if (Array.isArray(readPackageHook)) {
+    hooks.push(...readPackageHook)
+  } else if (readPackageHook) {
+    hooks.push(readPackageHook)
+  }
+
   if (hooks.length === 0) {
-    return readPackageHook
+    return undefined
   }
   const readPackageAndExtend = hooks.length === 1
     ? hooks[0]
-    : pipeWith(async (f, res) => f(await res), hooks as any) as ReadPackageHook // eslint-disable-line @typescript-eslint/no-explicit-any
-  if (readPackageHook != null) {
-    return (async (manifest: ProjectManifest, dir?: string) => readPackageAndExtend(await readPackageHook(manifest, dir), dir)) as ReadPackageHook
-  }
+    : ((pkg: PackageManifest | ProjectManifest, dir: string) => pipeWith(async (f, res) => f(await res, dir), hooks as any)(pkg)) as ReadPackageHook // eslint-disable-line @typescript-eslint/no-explicit-any
   return readPackageAndExtend
 }
 

packages/core/test/install/hooks.ts

@@ -83,3 +83,42 @@ test('readPackage, afterAllResolved async hooks', async () => {
   const wantedLockfile = await project.readLockfile()
   expect(wantedLockfile['foo']).toEqual('foo')
 })
+
+test('readPackage hooks array', async () => {
+  const project = prepareEmpty()
+
+  // w/o the hook, 100.1.0 would be installed
+  await addDistTag({ package: '@pnpm.e2e/dep-of-pkg-with-1-dep', version: '100.1.0', distTag: 'latest' })
+
+  function readPackageHook1 (manifest: PackageManifest) {
+    switch (manifest.name) {
+    case '@pnpm.e2e/pkg-with-1-dep':
+      if (manifest.dependencies == null) {
+        throw new Error('@pnpm.e2e/pkg-with-1-dep expected to have a dependencies field')
+      }
+      manifest.dependencies['@pnpm.e2e/dep-of-pkg-with-1-dep'] = '50.0.0'
+      break
+    }
+    return manifest
+  }
+
+  function readPackageHook2 (manifest: PackageManifest) {
+    switch (manifest.name) {
+    case '@pnpm.e2e/pkg-with-1-dep':
+      if (manifest.dependencies == null) {
+        throw new Error('@pnpm.e2e/pkg-with-1-dep expected to have a dependencies field')
+      }
+      manifest.dependencies['@pnpm.e2e/dep-of-pkg-with-1-dep'] = '100.0.0'
+      break
+    }
+    return manifest
+  }
+
+  await addDependenciesToPackage({}, ['@pnpm.e2e/pkg-with-1-dep'], await testDefaults({
+    hooks: {
+      readPackage: [readPackageHook1, readPackageHook2],
+    },
+  }))
+
+  await project.storeHas('@pnpm.e2e/dep-of-pkg-with-1-dep', '100.0.0')
+})

packages/plugin-commands-installation/src/recursive.ts

@@ -289,7 +289,17 @@ export default async function recursive (
   const limitInstallation = pLimit(opts.workspaceConcurrency ?? 4)
   await Promise.all(pkgPaths.map(async (rootDir: string) =>
     limitInstallation(async () => {
-      const hooks = opts.ignorePnpmfile ? {} : requireHooks(rootDir, opts)
+      const pnpmfileHooks = opts.ignorePnpmfile ? {} : requireHooks(rootDir, opts)
+      const optsHooks = opts.hooks ?? {}
+      const hooks = {
+        ...optsHooks,
+        ...pnpmfileHooks,
+        readPackage: pnpmfileHooks.readPackage
+          ? optsHooks.readPackage
+            ? [pnpmfileHooks.readPackage, optsHooks.readPackage]
+            : pnpmfileHooks.readPackage
+          : optsHooks.readPackage,
+      }
       try {
         if (opts.ignoredPackages?.has(rootDir)) {
           return

packages/plugin-commands-installation/test/miscRecursive.ts

@@ -715,3 +715,45 @@ test('installing in monorepo with shared lockfile should work on virtual drives'
 
   await projects['project-1'].has('is-positive')
 })
+
+test('pass readPackage with shared lockfile', async () => {
+  const projects = preparePackages([
+    {
+      name: 'project-1',
+      version: '1.0.0',
+      dependencies: {
+        'is-negative': '1.0.0',
+      },
+    },
+    {
+      name: 'project-2',
+      version: '1.0.0',
+      dependencies: {
+        'is-negative': '1.0.0',
+      },
+    },
+  ])
+
+  await install.handler({
+    ...DEFAULT_OPTS,
+    ...await readProjects(process.cwd(), []),
+    dir: process.cwd(),
+    recursive: true,
+    workspaceDir: process.cwd(),
+    hooks: {
+      readPackage (pkg) {
+        return {
+          ...pkg,
+          dependencies: {
+            'is-positive': '1.0.0',
+          },
+        }
+      },
+    },
+  })
+
+  await projects['project-1'].has('is-positive')
+  await projects['project-1'].hasNot('is-negative')
+  await projects['project-2'].has('is-positive')
+  await projects['project-2'].hasNot('is-negative')
+})