feat: support async hooks (#3955)

.changeset/fast-lies-exercise.md

@@ -0,0 +1,10 @@
+---
+"@pnpm/core": minor
+"@pnpm/get-context": minor
+"pnpm": minor
+"@pnpm/pnpmfile": minor
+"@pnpm/resolve-dependencies": minor
+"@pnpm/types": minor
+---
+
+Support async hooks

packages/core/README.md

@@ -72,10 +72,11 @@ Remove unreferenced packages from the store.
 
 Hooks are functions that can step into the installation process.
 
-### `readPackage(pkg)`
+### `readPackage(pkg: Manifest): Manifest | Promise<Manifest>`
 
 This hook is called with every dependency's manifest information.
 The modified manifest returned by this hook is then used by `@pnpm/core` during installation.
+An async function is supported.
 
 **Example:**
 
@@ -96,9 +97,10 @@ function readPackage (pkg) {
 }
 ```
 
-### `afterAllResolved(lockfile: Lockfile): Lockfile`
+### `afterAllResolved(lockfile: Lockfile): Lockfile | Promise<Lockfile>`
 
 This hook is called after all dependencies are resolved. It recieves and returns the resolved lockfile object.
+An async function is supported.
 
 ## License
 

packages/core/src/install/extendInstallOptions.ts

@@ -52,7 +52,7 @@ export interface StrictInstallOptions {
   pruneLockfileImporters: boolean
   hooks: {
     readPackage?: ReadPackageHook
-    afterAllResolved?: (lockfile: Lockfile) => Lockfile
+    afterAllResolved?: (lockfile: Lockfile) => Lockfile | Promise<Lockfile>
   }
   sideEffectsCacheRead: boolean
   sideEffectsCacheWrite: boolean

packages/core/src/install/index.ts

@@ -57,7 +57,7 @@ import flatten from 'ramda/src/flatten'
 import fromPairs from 'ramda/src/fromPairs'
 import equals from 'ramda/src/equals'
 import isEmpty from 'ramda/src/isEmpty'
-import pipe from 'ramda/src/pipe'
+import pipeWith from 'ramda/src/pipeWith'
 import props from 'ramda/src/props'
 import unnest from 'ramda/src/unnest'
 import parseWantedDependencies from '../parseWantedDependencies'
@@ -531,9 +531,9 @@ function createReadPackageHook (
   if (hooks.length === 0) {
     return readPackageHook
   }
-  const readPackageAndExtend = hooks.length === 1 ? hooks[0] : pipe(hooks[0], hooks[1]) as ReadPackageHook
+  const readPackageAndExtend = hooks.length === 1 ? hooks[0] : pipeWith(async (f, res) => f(await res), [hooks[0], hooks[1]]) as ReadPackageHook
   if (readPackageHook != null) {
-    return ((manifest: ProjectManifest, dir?: string) => readPackageAndExtend(readPackageHook(manifest, dir), dir)) as ReadPackageHook
+    return (async (manifest: ProjectManifest, dir?: string) => readPackageAndExtend(await readPackageHook(manifest, dir), dir)) as ReadPackageHook
   }
   return readPackageAndExtend
 }
@@ -804,7 +804,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
   })
 
   newLockfile = ((opts.hooks?.afterAllResolved) != null)
-    ? opts.hooks?.afterAllResolved(newLockfile)
+    ? await opts.hooks?.afterAllResolved(newLockfile)
     : newLockfile
 
   if (opts.updateLockfileMinorVersion) {

packages/core/test/install/hooks.ts

@@ -5,7 +5,6 @@ import {
   addDependenciesToPackage,
   PackageManifest,
 } from '@pnpm/core'
-import sinon from 'sinon'
 import {
   addDistTag,
   testDefaults,
@@ -29,7 +28,7 @@ test('readPackage, afterAllResolved hooks', async () => {
     return manifest
   }
 
-  const afterAllResolved = sinon.spy((lockfile: Lockfile) => {
+  const afterAllResolved = jest.fn((lockfile: Lockfile) => {
     lockfile['foo'] = 'foo' // eslint-disable-line
     return lockfile
   })
@@ -42,8 +41,46 @@ test('readPackage, afterAllResolved hooks', async () => {
   }))
 
   await project.storeHas('dep-of-pkg-with-1-dep', '100.0.0')
-  expect(afterAllResolved.calledOnce).toBeTruthy()
-  expect(afterAllResolved.getCall(0).args[0].lockfileVersion).toEqual(LOCKFILE_VERSION)
+  expect(afterAllResolved).toHaveBeenCalledTimes(1)
+  expect(afterAllResolved.mock.calls[0][0].lockfileVersion).toEqual(LOCKFILE_VERSION)
+
+  const wantedLockfile = await project.readLockfile()
+  expect(wantedLockfile['foo']).toEqual('foo') // eslint-disable-line @typescript-eslint/dot-notation
+})
+
+test('readPackage, afterAllResolved async hooks', async () => {
+  const project = prepareEmpty()
+
+  // w/o the hook, 100.1.0 would be installed
+  await addDistTag('dep-of-pkg-with-1-dep', '100.1.0', 'latest')
+
+  async function readPackageHook (manifest: PackageManifest) {
+    switch (manifest.name) {
+    case 'pkg-with-1-dep':
+      if (manifest.dependencies == null) {
+        throw new Error('pkg-with-1-dep expected to have a dependencies field')
+      }
+      manifest.dependencies['dep-of-pkg-with-1-dep'] = '100.0.0'
+      break
+    }
+    return manifest
+  }
+
+  const afterAllResolved = jest.fn(async (lockfile: Lockfile) => {
+    lockfile['foo'] = 'foo' // eslint-disable-line
+    return lockfile
+  })
+
+  await addDependenciesToPackage({}, ['pkg-with-1-dep'], await testDefaults({
+    hooks: {
+      afterAllResolved,
+      readPackage: readPackageHook,
+    },
+  }))
+
+  await project.storeHas('dep-of-pkg-with-1-dep', '100.0.0')
+  expect(afterAllResolved).toHaveBeenCalledTimes(1)
+  expect(afterAllResolved.mock.calls[0][0].lockfileVersion).toEqual(LOCKFILE_VERSION)
 
   const wantedLockfile = await project.readLockfile()
   expect(wantedLockfile['foo']).toEqual('foo') // eslint-disable-line @typescript-eslint/dot-notation

packages/get-context/src/index.ts

@@ -132,7 +132,7 @@ export default async function getContext<T> (
   if ((opts.hooks?.readPackage) != null) {
     for (const project of importersContext.projects) {
       project.originalManifest = project.manifest
-      project.manifest = opts.hooks.readPackage(clone(project.manifest), project.rootDir)
+      project.manifest = await opts.hooks.readPackage(clone(project.manifest), project.rootDir)
     }
   }
 
@@ -447,7 +447,7 @@ export async function getContextForSingleImporter (
     importerId,
     include: opts.include ?? include,
     lockfileDir: opts.lockfileDir,
-    manifest: opts.hooks?.readPackage?.(manifest) ?? manifest,
+    manifest: await opts.hooks?.readPackage?.(manifest) ?? manifest,
     modulesDir,
     modulesFile: modules,
     pendingBuilds,

packages/pnpm/test/install/hooks.ts

@@ -35,6 +35,31 @@ test('readPackage hook', async () => {
   await project.storeHas('dep-of-pkg-with-1-dep', '100.0.0')
 })
 
+test('readPackage async hook', async () => {
+  const project = prepare()
+
+  await fs.writeFile('.pnpmfile.cjs', `
+    'use strict'
+    module.exports = {
+      hooks: {
+        async readPackage (pkg) {
+          if (pkg.name === 'pkg-with-1-dep') {
+            pkg.dependencies['dep-of-pkg-with-1-dep'] = '100.0.0'
+          }
+          return pkg
+        }
+      }
+    }
+  `, 'utf8')
+
+  // w/o the hook, 100.1.0 would be installed
+  await addDistTag('dep-of-pkg-with-1-dep', '100.1.0', 'latest')
+
+  await execPnpm(['install', 'pkg-with-1-dep'])
+
+  await project.storeHas('dep-of-pkg-with-1-dep', '100.0.0')
+})
+
 test('readPackage hook makes installation fail if it does not return the modified package manifests', async () => {
   prepare()
 
@@ -143,6 +168,47 @@ test('readPackage hook from global pnpmfile and local pnpmfile', async () => {
   await project.storeHas('is-positive', '1.0.0')
 })
 
+test('readPackage async hook from global pnpmfile and local pnpmfile', async () => {
+  const project = prepare()
+
+  await fs.writeFile('../.pnpmfile.cjs', `
+    'use strict'
+    module.exports = {
+      hooks: {
+        async readPackage (pkg) {
+          if (pkg.name === 'pkg-with-1-dep') {
+            pkg.dependencies['dep-of-pkg-with-1-dep'] = '100.0.0'
+            pkg.dependencies['is-positive'] = '3.0.0'
+          }
+          return pkg
+        }
+      }
+    }
+  `, 'utf8')
+
+  await fs.writeFile('.pnpmfile.cjs', `
+    'use strict'
+    module.exports = {
+      hooks: {
+        async readPackage (pkg) {
+          if (pkg.name === 'pkg-with-1-dep') {
+            pkg.dependencies['is-positive'] = '1.0.0'
+          }
+          return pkg
+        }
+      }
+    }
+  `, 'utf8')
+
+  // w/o the hook, 100.1.0 would be installed
+  await addDistTag('dep-of-pkg-with-1-dep', '100.1.0', 'latest')
+
+  await execPnpm(['install', 'pkg-with-1-dep', '--global-pnpmfile', path.resolve('..', '.pnpmfile.cjs')])
+
+  await project.storeHas('dep-of-pkg-with-1-dep', '100.0.0')
+  await project.storeHas('is-positive', '1.0.0')
+})
+
 test('readPackage hook from pnpmfile at root of workspace', async () => {
   const projects = preparePackages([
     {
@@ -426,6 +492,36 @@ test('pnpmfile: run afterAllResolved hook', async () => {
   expect(hookLog.message).toBe('All resolved')
 })
 
+test('pnpmfile: run async afterAllResolved hook', async () => {
+  prepare()
+
+  await fs.writeFile('.pnpmfile.cjs', `
+    'use strict'
+    module.exports = {
+      hooks: {
+        async afterAllResolved (lockfile, context) {
+          context.log('All resolved')
+          return lockfile
+        }
+      }
+    }
+  `, 'utf8')
+
+  const proc = execPnpmSync(['install', 'pkg-with-1-dep', '--reporter', 'ndjson'])
+
+  const outputs = proc.stdout.toString().split(/\r?\n/)
+
+  const hookLog = outputs.filter(Boolean)
+    .map((output) => JSON.parse(output))
+    .find((log) => log.name === 'pnpm:hook')
+
+  expect(hookLog).toBeTruthy()
+  expect(hookLog.prefix).toBeTruthy()
+  expect(hookLog.from).toBeTruthy()
+  expect(hookLog.hook).toBe('afterAllResolved')
+  expect(hookLog.message).toBe('All resolved')
+})
+
 test('readPackage hook normalizes the package manifest', async () => {
   prepare()
 

packages/pnpmfile/src/requireHooks.ts

@@ -13,7 +13,7 @@ interface HookContext {
 interface Hooks {
   // eslint-disable-next-line
   readPackage?: (pkg: any, context: HookContext) => any
-  afterAllResolved?: (lockfile: Lockfile, context: HookContext) => Lockfile
+  afterAllResolved?: (lockfile: Lockfile, context: HookContext) => Lockfile | Promise<Lockfile>
   filterLog?: (log: Log) => boolean
 }
 
@@ -59,9 +59,9 @@ export default function requireHooks (
       const globalHookContext = createReadPackageHookContext(globalPnpmfile.filename, prefix, hookName)
       const localHookContext = createReadPackageHookContext(pnpmFile.filename, prefix, hookName)
       // the `arg` is a package manifest in case of readPackage() and a lockfile object in case of afterAllResolved()
-      cookedHooks[hookName] = (arg: object) => {
+      cookedHooks[hookName] = async (arg: object) => {
         return hooks[hookName](
-          globalHooks[hookName](arg, globalHookContext),
+          await globalHooks[hookName](arg, globalHookContext),
           localHookContext
         )
       }

packages/pnpmfile/src/requirePnpmfile.ts

@@ -43,12 +43,12 @@ export default (pnpmFilePath: string, prefix: string) => {
     }
     if (pnpmfile?.hooks?.readPackage) {
       const readPackage = pnpmfile.hooks.readPackage
-      pnpmfile.hooks.readPackage = function (pkg: PackageManifest, ...args: any[]) { // eslint-disable-line
+      pnpmfile.hooks.readPackage = async function (pkg: PackageManifest, ...args: any[]) { // eslint-disable-line
         pkg.dependencies = pkg.dependencies ?? {}
         pkg.devDependencies = pkg.devDependencies ?? {}
         pkg.optionalDependencies = pkg.optionalDependencies ?? {}
         pkg.peerDependencies = pkg.peerDependencies ?? {}
-        const newPkg = readPackage(pkg, ...args)
+        const newPkg = await readPackage(pkg, ...args)
         if (!newPkg) {
           throw new BadReadPackageHookError(pnpmFilePath, 'readPackage hook did not return a package manifest object.')
         }

packages/pnpmfile/test/index.ts

@@ -10,17 +10,17 @@ test('readPackage hook run fails when returns undefined ', () => {
   const pnpmfilePath = path.join(__dirname, 'pnpmfiles/readPackageNoReturn.js')
   const pnpmfile = requirePnpmfile(pnpmfilePath, __dirname)
 
-  expect(() => {
+  return expect(
     pnpmfile.hooks.readPackage({})
-  }).toThrow(new BadReadPackageHookError(pnpmfilePath, 'readPackage hook did not return a package manifest object.'))
+  ).rejects.toEqual(new BadReadPackageHookError(pnpmfilePath, 'readPackage hook did not return a package manifest object.'))
 })
 
 test('readPackage hook run fails when returned dependencies is not an object ', () => {
   const pnpmfilePath = path.join(__dirname, 'pnpmfiles/readPackageNoObject.js')
   const pnpmfile = requirePnpmfile(pnpmfilePath, __dirname)
-  expect(() => {
+  return expect(
     pnpmfile.hooks.readPackage({})
-  }).toThrow(new BadReadPackageHookError(pnpmfilePath, 'readPackage hook returned package manifest object\'s property \'dependencies\' must be an object.'))
+  ).rejects.toEqual(new BadReadPackageHookError(pnpmfilePath, 'readPackage hook returned package manifest object\'s property \'dependencies\' must be an object.'))
 })
 
 test('filterLog hook combines with the global hook', () => {

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -686,7 +686,7 @@ async function resolveDependency (
   let prepare!: boolean
   let hasBin!: boolean
   pkg = (ctx.readPackageHook != null)
-    ? ctx.readPackageHook(pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!())
+    ? await ctx.readPackageHook(pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!())
     : pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!()
   if (!pkg.name) { // TODO: don't fail on optional dependencies
     throw new PnpmError('MISSING_PACKAGE_NAME', `Can't install ${wantedDependency.pref}: Missing package name`)

packages/types/src/options.ts

@@ -14,6 +14,6 @@ export type IncludedDependencies = {
 }
 
 export interface ReadPackageHook {
-  (pkg: PackageManifest, dir?: string): PackageManifest
-  (pkg: ProjectManifest, dir?: string): ProjectManifest
+  (pkg: PackageManifest, dir?: string): PackageManifest | Promise<PackageManifest>
+  (pkg: ProjectManifest, dir?: string): ProjectManifest | Promise<ProjectManifest>
 }