feat: merge readPackage hook from opts and pnpmfile (#5403)

Close #5306

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/breezy-weeks-rescue.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-deploy": patch
+"pnpm": patch
+---
+
+Hooks should be applied on `pnpm deploy` [#5306](https://github.com/pnpm/pnpm/issues/5306).

.changeset/curvy-socks-press.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/hooks.read-package-hook": major
+---
+
+First release.

.changeset/plenty-terms-count.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/core": major
+"@pnpm/default-reporter": major
+---
+
+Accept an array of hooks.

.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

.changeset/silly-nails-remember.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/pnpmfile": major
+---
+
+Returns an array of hook functions.

.changeset/tender-radios-hug.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/get-context": major
+---
+
+Pass readPackageHook as a separate option not as a subproperty of `hooks`.

packages/core/package.json

@@ -26,6 +26,7 @@
     "@pnpm/graph-sequencer": "1.0.0",
     "@pnpm/headless": "workspace:*",
     "@pnpm/hoist": "workspace:*",
+    "@pnpm/hooks.read-package-hook": "workspace:*",
     "@pnpm/lifecycle": "workspace:*",
     "@pnpm/link-bins": "workspace:*",
     "@pnpm/lockfile-file": "workspace:*",
@@ -39,7 +40,6 @@
     "@pnpm/normalize-registries": "workspace:*",
     "@pnpm/npm-package-arg": "^1.0.0",
     "@pnpm/package-requester": "workspace:*",
-    "@pnpm/parse-overrides": "workspace:*",
     "@pnpm/parse-wanted-dependency": "workspace:*",
     "@pnpm/prune-lockfile": "workspace:*",
     "@pnpm/read-modules-dir": "workspace:*",
@@ -52,7 +52,6 @@
     "@pnpm/symlink-dependency": "workspace:*",
     "@pnpm/types": "workspace:*",
     "@pnpm/which-version-is-pinned": "workspace:*",
-    "@yarnpkg/extensions": "1.2.0-rc.1",
     "@zkochan/rimraf": "^2.1.2",
     "dependency-path": "workspace:*",
     "is-inner-link": "^4.0.0",

packages/core/src/getPeerDependencyIssues.ts

@@ -1,7 +1,7 @@
 import resolveDependencies, { getWantedDependencies } from '@pnpm/resolve-dependencies'
 import { PeerDependencyIssuesByProjects } from '@pnpm/types'
 import getContext, { GetContextOptions, ProjectOptions } from '@pnpm/get-context'
-import { createReadPackageHook } from './install'
+import { createReadPackageHook } from '@pnpm/hooks.read-package-hook'
 import { getPreferredVersionsFromLockfile } from './install/getPreferredVersions'
 import { InstallOptions } from './install/extendInstallOptions'
 import { DEFAULT_REGISTRIES } from '@pnpm/normalize-registries'

packages/core/src/install/extendInstallOptions.ts

@@ -1,6 +1,7 @@
 import { WANTED_LOCKFILE } from '@pnpm/constants'
 import PnpmError from '@pnpm/error'
 import { HoistingLimits } from '@pnpm/headless'
+import { createReadPackageHook } from '@pnpm/hooks.read-package-hook'
 import { Lockfile } from '@pnpm/lockfile-file'
 import { IncludedDependencies } from '@pnpm/modules-yaml'
 import normalizeRegistries, { DEFAULT_REGISTRIES } from '@pnpm/normalize-registries'
@@ -71,9 +72,9 @@ export interface StrictInstallOptions {
   }
   pruneLockfileImporters: boolean
   hooks: {
-    readPackage?: ReadPackageHook
+    readPackage?: ReadPackageHook[]
     preResolution?: (ctx: PreResolutionHookContext) => Promise<void>
-    afterAllResolved?: (lockfile: Lockfile) => Lockfile | Promise<Lockfile>
+    afterAllResolved?: Array<(lockfile: Lockfile) => Lockfile | Promise<Lockfile>>
   }
   sideEffectsCacheRead: boolean
   sideEffectsCacheWrite: boolean
@@ -198,9 +199,13 @@ const defaults = async (opts: InstallOptions) => {
   } as StrictInstallOptions
 }
 
+export type ProcessedInstallOptions = StrictInstallOptions & {
+  readPackageHook?: ReadPackageHook
+}
+
 export default async (
   opts: InstallOptions
-): Promise<StrictInstallOptions> => {
+): Promise<ProcessedInstallOptions> => {
   if (opts) {
     for (const key in opts) {
       if (opts[key] === undefined) {
@@ -212,11 +217,19 @@ export default async (
     throw new PnpmError('CONFIG_CONFLICT_BUILT_DEPENDENCIES', 'Cannot have both neverBuiltDependencies and onlyBuiltDependencies')
   }
   const defaultOpts = await defaults(opts)
-  const extendedOpts = {
+  const extendedOpts: ProcessedInstallOptions = {
     ...defaultOpts,
     ...opts,
     storeDir: defaultOpts.storeDir,
   }
+  extendedOpts.readPackageHook = createReadPackageHook({
+    ignoreCompatibilityDb: extendedOpts.ignoreCompatibilityDb,
+    readPackageHook: extendedOpts.hooks?.readPackage,
+    overrides: extendedOpts.overrides,
+    lockfileDir: extendedOpts.lockfileDir,
+    packageExtensions: extendedOpts.packageExtensions,
+    peerDependencyRules: extendedOpts.peerDependencyRules,
+  })
   if (extendedOpts.lockfileOnly) {
     extendedOpts.ignoreScripts = true
     if (!extendedOpts.useLockfile) {

packages/core/src/install/index.ts

@@ -50,13 +50,10 @@ import {
 import {
   DependenciesField,
   DependencyManifest,
-  PackageExtension,
   PeerDependencyIssues,
-  PeerDependencyRules,
   ProjectManifest,
   ReadPackageHook,
 } from '@pnpm/types'
-import { packageExtensions as compatPackageExtensions } from '@yarnpkg/extensions'
 import rimraf from '@zkochan/rimraf'
 import isInnerLink from 'is-inner-link'
 import pFilter from 'p-filter'
@@ -71,12 +68,9 @@ import unnest from 'ramda/src/unnest'
 import parseWantedDependencies from '../parseWantedDependencies'
 import removeDeps from '../uninstall/removeDeps'
 import allProjectsAreUpToDate from './allProjectsAreUpToDate'
-import createPackageExtender from './createPackageExtender'
-import createVersionsOverrider from './createVersionsOverrider'
-import createPeerDependencyPatcher from './createPeerDependencyPatcher'
 import extendOptions, {
   InstallOptions,
-  StrictInstallOptions,
+  ProcessedInstallOptions as StrictInstallOptions,
 } from './extendInstallOptions'
 import { getPreferredVersionsFromLockfile, getAllUniqueSpecs } from './getPreferredVersions'
 import linkPackages from './link'
@@ -152,6 +146,9 @@ export type MutatedProject = ProjectOptions & DependenciesMutation
 
 export type MutateModulesOptions = InstallOptions & {
   preferredVersions?: PreferredVersions
+  hooks?: {
+    readPackage?: ReadPackageHook[] | ReadPackageHook
+  } | InstallOptions['hooks']
 }
 
 export async function mutateModules (
@@ -176,14 +173,6 @@ export async function mutateModules (
     // When running install/update on a subset of projects, the root project might not be included,
     // so reading its manifest explicitly here.
     await safeReadProjectManifestOnly(opts.lockfileDir)
-  opts.hooks.readPackage = createReadPackageHook({
-    ignoreCompatibilityDb: opts.ignoreCompatibilityDb,
-    readPackageHook: opts.hooks.readPackage,
-    overrides: opts.overrides,
-    lockfileDir: opts.lockfileDir,
-    packageExtensions: opts.packageExtensions,
-    peerDependencyRules: opts.peerDependencyRules,
-  })
 
   const ctx = await getContext(projects, opts)
 
@@ -571,55 +560,6 @@ export function createObjectChecksum (obj: Object) {
   return crypto.createHash('md5').update(s).digest('hex')
 }
 
-export function createReadPackageHook (
-  {
-    ignoreCompatibilityDb,
-    lockfileDir,
-    overrides,
-    packageExtensions,
-    peerDependencyRules,
-    readPackageHook,
-  }: {
-    ignoreCompatibilityDb?: boolean
-    lockfileDir: string
-    overrides?: Record<string, string>
-    packageExtensions?: Record<string, PackageExtension>
-    peerDependencyRules?: PeerDependencyRules
-    readPackageHook?: ReadPackageHook
-  }
-): ReadPackageHook | undefined {
-  const hooks: ReadPackageHook[] = []
-  if (!isEmpty(overrides ?? {})) {
-    hooks.push(createVersionsOverrider(overrides!, lockfileDir))
-  }
-  if (!ignoreCompatibilityDb) {
-    hooks.push(createPackageExtender(fromPairs(compatPackageExtensions)))
-  }
-  if (!isEmpty(packageExtensions ?? {})) {
-    hooks.push(createPackageExtender(packageExtensions!))
-  }
-  if (
-    peerDependencyRules != null &&
-    (
-      !isEmpty(peerDependencyRules.ignoreMissing) ||
-      !isEmpty(peerDependencyRules.allowedVersions) ||
-      !isEmpty(peerDependencyRules.allowAny)
-    )
-  ) {
-    hooks.push(createPeerDependencyPatcher(peerDependencyRules))
-  }
-  if (hooks.length === 0) {
-    return readPackageHook
-  }
-  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
-  }
-  return readPackageAndExtend
-}
-
 function cacheExpired (prunedAt: string, maxAgeInMinutes: number) {
   return ((Date.now() - new Date(prunedAt).valueOf()) / (1000 * 60)) > maxAgeInMinutes
 }
@@ -827,7 +767,9 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
       engineStrict: opts.engineStrict,
       force: opts.force,
       forceFullResolution,
-      hooks: opts.hooks,
+      hooks: {
+        readPackage: opts.readPackageHook,
+      },
       linkWorkspacePackagesDepth: opts.linkWorkspacePackagesDepth ?? (opts.saveWorkspaceProtocol ? 0 : -1),
       lockfileDir: opts.lockfileDir,
       nodeVersion: opts.nodeVersion,
@@ -882,7 +824,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
   })
 
   newLockfile = ((opts.hooks?.afterAllResolved) != null)
-    ? await opts.hooks?.afterAllResolved(newLockfile)
+    ? await pipeWith(async (f, res) => f(await res), opts.hooks.afterAllResolved as any)(newLockfile) as Lockfile // eslint-disable-line
     : newLockfile
 
   if (opts.updateLockfileMinorVersion) {

packages/core/test/install/hooks.ts

@@ -33,8 +33,8 @@ test('readPackage, afterAllResolved hooks', async () => {
 
   await addDependenciesToPackage({}, ['@pnpm.e2e/pkg-with-1-dep'], await testDefaults({
     hooks: {
-      afterAllResolved,
-      readPackage: readPackageHook,
+      afterAllResolved: [afterAllResolved],
+      readPackage: [readPackageHook],
     },
   }))
 
@@ -71,8 +71,8 @@ test('readPackage, afterAllResolved async hooks', async () => {
 
   await addDependenciesToPackage({}, ['@pnpm.e2e/pkg-with-1-dep'], await testDefaults({
     hooks: {
-      afterAllResolved,
-      readPackage: readPackageHook,
+      afterAllResolved: [afterAllResolved],
+      readPackage: [readPackageHook],
     },
   }))
 
@@ -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/core/tsconfig.json

@@ -63,6 +63,9 @@
     {
       "path": "../hoist"
     },
+    {
+      "path": "../hooks.read-package-hook"
+    },
     {
       "path": "../lifecycle"
     },
@@ -102,9 +105,6 @@
     {
       "path": "../package-store"
     },
-    {
-      "path": "../parse-overrides"
-    },
     {
       "path": "../parse-wanted-dependency"
     },

packages/default-reporter/src/index.ts

@@ -194,7 +194,11 @@ export function toOutput$ (
   }, 0)
   let other = Rx.from(otherPushStream)
   if (opts.context.config?.hooks?.filterLog != null) {
-    other = other.pipe(filter(opts.context.config.hooks.filterLog))
+    const filterLogs = opts.context.config.hooks.filterLog
+    const filterFn = filterLogs.length === 1
+      ? filterLogs[0]
+      : (log: logs.Log) => filterLogs.every!((filterLog) => filterLog(log))
+    other = other.pipe(filter(filterFn))
   }
   const log$ = {
     context: Rx.from(contextPushStream),

packages/default-reporter/test/filterLogHook.ts

@@ -8,7 +8,7 @@ test('logger with filterLog hook', (done) => {
       argv: ['install'],
       config: {
         hooks: {
-          filterLog: (log: Log) => {
+          filterLog: [(log: Log) => {
             if (log.level === 'debug') {
               return false
             }
@@ -21,7 +21,7 @@ test('logger with filterLog hook', (done) => {
               }
             }
             return true
-          },
+          }],
         },
       } as any, // eslint-disable-line
     },
@@ -61,4 +61,4 @@ test('logger with filterLog hook', (done) => {
     done()
     subscription.unsubscribe()
   }, 10)
-})
+})