From c074cf7b1abdcdd2c07d9e07b96459b14d7eba0f Mon Sep 17 00:00:00 2001 From: Grzegorz Abramczyk Date: Mon, 5 Sep 2022 23:24:21 +0200 Subject: [PATCH] fix: implement merging hooks provided in opts Pnpm deploy command is providing readPackage in opts and it is being discarded by requireHooks. This change ensures, that hooks provided in opts are merged with those from local and global pnpmfiles. Closes #5306 --- packages/pnpmfile/src/requireHooks.ts | 71 ++++++++---- packages/pnpmfile/test/index.ts | 103 ++++++++++++++++++ packages/pnpmfile/test/pnpmfiles/filterLog.js | 2 +- .../test/pnpmfiles/globalFilterLog.js | 2 +- 4 files changed, 156 insertions(+), 22 deletions(-) diff --git a/packages/pnpmfile/src/requireHooks.ts b/packages/pnpmfile/src/requireHooks.ts index 62ef34635e3..3888f6e8452 100644 --- a/packages/pnpmfile/src/requireHooks.ts +++ b/packages/pnpmfile/src/requireHooks.ts @@ -43,6 +43,7 @@ export default function requireHooks ( opts: { globalPnpmfile?: string pnpmfile?: string + hooks?: Hooks } ): CookedHooks { const globalPnpmfile = opts.globalPnpmfile && requirePnpmfile(pathAbsolute(opts.globalPnpmfile, prefix), prefix) @@ -51,38 +52,68 @@ export default function requireHooks ( const pnpmFile = opts.pnpmfile && requirePnpmfile(pathAbsolute(opts.pnpmfile, prefix), prefix) || requirePnpmfile(path.join(prefix, '.pnpmfile.cjs'), prefix) let hooks: Hooks = pnpmFile?.hooks + let optsHooks = opts?.hooks as Hooks - if (!globalHooks && !hooks) return {} + if (!globalHooks && !hooks && !optsHooks) return {} globalHooks = globalHooks || {} hooks = hooks || {} + optsHooks = optsHooks || {} const cookedHooks: CookedHooks = {} for (const hookName of ['readPackage', 'afterAllResolved']) { - if (globalHooks[hookName] && hooks[hookName]) { + // eslint-disable-next-line + const hookStack: Array<(arg: object) => Promise> = [] + if (globalHooks[hookName]) { const globalHookContext = createReadPackageHookContext(globalPnpmfile.filename, prefix, hookName) + hookStack.push(async (arg: object) => { + return globalHooks[hookName](arg, globalHookContext) + }) + } + if (hooks[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() + hookStack.push(async (arg: object) => { + return hooks[hookName](arg, localHookContext) + }) + } + if (optsHooks[hookName]) { + const optsHookContext = createReadPackageHookContext('opts', prefix, hookName) + hookStack.push(async (arg: object) => { + return optsHooks[hookName](arg, optsHookContext) + }) + } + if (hookStack.length === 3) { cookedHooks[hookName] = async (arg: object) => { - return hooks[hookName]( - await globalHooks[hookName](arg, globalHookContext), - localHookContext + return hookStack[2]( + await hookStack[1]( + await hookStack[0](arg) + ) ) } - } else if (globalHooks[hookName]) { - const globalHook = globalHooks[hookName] - const context = createReadPackageHookContext(globalPnpmfile.filename, prefix, hookName) - cookedHooks[hookName] = (pkg: object) => globalHook(pkg, context) - } else if (hooks[hookName]) { - const hook = hooks[hookName] - const context = createReadPackageHookContext(pnpmFile.filename, prefix, hookName) - cookedHooks[hookName] = (pkg: object) => hook(pkg, context) + } else if (hookStack.length === 2) { + cookedHooks[hookName] = async (arg: object) => { + return hookStack[1]( + await hookStack[0](arg) + ) + } + } else if (hookStack.length === 1) { + cookedHooks[hookName] = hookStack[0] } } - const globalFilterLog = globalHooks.filterLog - const filterLog = hooks.filterLog - if (globalFilterLog != null && filterLog != null) { - cookedHooks.filterLog = (log: Log) => globalFilterLog(log) && filterLog(log) - } else { - cookedHooks.filterLog = globalFilterLog ?? filterLog + const filterLogStack: Array<(log: Log) => boolean> = [] + if (globalHooks.filterLog) { + filterLogStack.push(globalHooks.filterLog) + } + if (hooks.filterLog) { + filterLogStack.push(hooks.filterLog) + } + if (optsHooks.filterLog) { + filterLogStack.push(optsHooks.filterLog) + } + if (filterLogStack.length === 3) { + cookedHooks.filterLog = (log: Log) => filterLogStack[0](log) && filterLogStack[1](log) && filterLogStack[2](log) + } if (filterLogStack.length === 2) { + cookedHooks.filterLog = (log: Log) => filterLogStack[0](log) && filterLogStack[1](log) + } if (filterLogStack.length === 1) { + cookedHooks.filterLog = filterLogStack[0] } // `importPackage`, `preResolution` and `fetchers` can only be defined via a global pnpmfile diff --git a/packages/pnpmfile/test/index.ts b/packages/pnpmfile/test/index.ts index 55414cd0f11..5034dca9254 100644 --- a/packages/pnpmfile/test/index.ts +++ b/packages/pnpmfile/test/index.ts @@ -23,6 +23,29 @@ test('readPackage hook run fails when returned dependencies is not an object ', ).rejects.toEqual(new BadReadPackageHookError(pnpmfilePath, 'readPackage hook returned package manifest object\'s property \'dependencies\' must be an object.')) }) +test('filterLog hook works from single source', () => { + const pnpmfile = path.join(__dirname, 'pnpmfiles/filterLog.js') + const hooks = requireHooks(__dirname, { pnpmfile }) + + expect(hooks.filterLog).toBeDefined() + expect(hooks.filterLog!({ + name: 'pnpm:summary', + level: 'error', + prefix: 'test', + })).toBeTruthy() + expect(hooks.filterLog!({ + name: 'pnpm:summary', + level: 'warn', + prefix: 'test', + message: 'message', + })).toBeTruthy() + expect(hooks.filterLog!({ + name: 'pnpm:summary', + level: 'debug', + prefix: 'test', + })).toBeTruthy() +}) + test('filterLog hook combines with the global hook', () => { const globalPnpmfile = path.join(__dirname, 'pnpmfiles/globalFilterLog.js') const pnpmfile = path.join(__dirname, 'pnpmfiles/filterLog.js') @@ -34,9 +57,89 @@ test('filterLog hook combines with the global hook', () => { level: 'error', prefix: 'test', })).toBeTruthy() + expect(hooks.filterLog!({ + name: 'pnpm:summary', + level: 'warn', + prefix: 'test', + message: 'message', + })).toBeTruthy() expect(hooks.filterLog!({ name: 'pnpm:summary', level: 'debug', prefix: 'test', })).toBeFalsy() }) + +test('filterLog hook combines with the global hook and the options hook', () => { + const globalPnpmfile = path.join(__dirname, 'pnpmfiles/globalFilterLog.js') + const pnpmfile = path.join(__dirname, 'pnpmfiles/filterLog.js') + const hooks = requireHooks(__dirname, { + globalPnpmfile, + pnpmfile, + hooks: { + filterLog (log) { + return log.level === 'error' + }, + }, + }) + + expect(hooks.filterLog).toBeDefined() + expect(hooks.filterLog!({ + name: 'pnpm:summary', + level: 'error', + prefix: 'test', + })).toBeTruthy() + expect(hooks.filterLog!({ + name: 'pnpm:summary', + level: 'warn', + prefix: 'test', + message: 'message', + })).toBeFalsy() + expect(hooks.filterLog!({ + name: 'pnpm:summary', + level: 'debug', + prefix: 'test', + })).toBeFalsy() +}) + +test('readPackage hook works from single source', async () => { + const pnpmfile = path.join(__dirname, 'pnpmfiles/readPackage.js') + const hooks = requireHooks(__dirname, { pnpmfile }) + + expect(hooks.readPackage).toBeDefined() + console.log(hooks.readPackage!({})) + expect(await hooks.readPackage!({})).toHaveProperty('local', true) + expect(await hooks.readPackage!({})).not.toHaveProperty('global', true) + expect(await hooks.readPackage!({})).not.toHaveProperty('opts', true) +}) + +test('readPackage hook combines with the global hook', async () => { + const pnpmfile = path.join(__dirname, 'pnpmfiles/readPackage.js') + const globalPnpmfile = path.join(__dirname, 'pnpmfiles/globalReadPackage.js') + const hooks = requireHooks(__dirname, { pnpmfile, globalPnpmfile }) + + expect(hooks.readPackage).toBeDefined() + expect(await hooks.readPackage!({})).toHaveProperty('local', true) + expect(await hooks.readPackage!({})).toHaveProperty('global', true) + expect(await hooks.readPackage!({})).not.toHaveProperty('opts', true) +}) + +test('readPackage hook combines with the global hook and the options hook', async () => { + const pnpmfile = path.join(__dirname, 'pnpmfiles/readPackage.js') + const globalPnpmfile = path.join(__dirname, 'pnpmfiles/globalReadPackage.js') + const hooks = requireHooks(__dirname, { + pnpmfile, + globalPnpmfile, + hooks: { + readPackage (pkg) { + pkg.opts = true + return pkg + }, + }, + }) + + expect(hooks.readPackage).toBeDefined() + expect(await hooks.readPackage!({})).toHaveProperty('local', true) + expect(await hooks.readPackage!({})).toHaveProperty('global', true) + expect(await hooks.readPackage!({})).toHaveProperty('opts', true) +}) \ No newline at end of file diff --git a/packages/pnpmfile/test/pnpmfiles/filterLog.js b/packages/pnpmfile/test/pnpmfiles/filterLog.js index e7eeffb2d97..308b946af19 100644 --- a/packages/pnpmfile/test/pnpmfiles/filterLog.js +++ b/packages/pnpmfile/test/pnpmfiles/filterLog.js @@ -3,5 +3,5 @@ module.exports = { } function filterLog(log) { - return log.level === 'debug' || log.level === 'error' + return log.level === 'debug' || log.level === 'error' || log.level === 'warn' } diff --git a/packages/pnpmfile/test/pnpmfiles/globalFilterLog.js b/packages/pnpmfile/test/pnpmfiles/globalFilterLog.js index c994b09fbfb..d3c6974ef3e 100644 --- a/packages/pnpmfile/test/pnpmfiles/globalFilterLog.js +++ b/packages/pnpmfile/test/pnpmfiles/globalFilterLog.js @@ -3,5 +3,5 @@ module.exports = { } function filterLog(log) { - return log.level === 'error' + return log.level === 'error' || log.level === 'warn' }