diff --git a/.changeset/deprecate-import-package-hook.md b/.changeset/deprecate-import-package-hook.md new file mode 100644 index 0000000000..1779bfcb05 --- /dev/null +++ b/.changeset/deprecate-import-package-hook.md @@ -0,0 +1,6 @@ +--- +"@pnpm/hooks.pnpmfile": patch +"pnpm": minor +--- + +The `importPackage` pnpmfile hook is deprecated. pnpm now prints a warning when a pnpmfile defines it, and the hook will be removed in the next major version. It also opts the installation out of the parallel package importer, making installation slower. If you rely on this hook, comment on [#14101](https://github.com/pnpm/pnpm/issues/14101). diff --git a/pnpm11/hooks/pnpmfile/src/requireHooks.ts b/pnpm11/hooks/pnpmfile/src/requireHooks.ts index 3dab760daf..951a5ab34c 100644 --- a/pnpm11/hooks/pnpmfile/src/requireHooks.ts +++ b/pnpm11/hooks/pnpmfile/src/requireHooks.ts @@ -1,8 +1,9 @@ import { hookLogger } from '@pnpm/core-loggers' import { createHashFromMultipleFiles } from '@pnpm/crypto.hash' -import { PnpmError } from '@pnpm/error' +import { PnpmError, redactAndSanitize } from '@pnpm/error' import type { CustomFetcher, CustomResolver, PreResolutionHookContext, PreResolutionHookLogger } from '@pnpm/hooks.types' import type { LockfileObject } from '@pnpm/lockfile.types' +import { globalWarn } from '@pnpm/logger' import type { ImportIndexedPackageAsync } from '@pnpm/store.controller-types' import type { BaseManifest, BeforePackingHook, ReadPackageHook } from '@pnpm/types' import { pathAbsolute } from 'path-absolute' @@ -226,6 +227,10 @@ export async function requireHooks ( } importProvider = file cookedHooks.importPackage = fileHooks.importPackage + globalWarn( + `The "importPackage" hook (defined in ${redactAndSanitize(file)}) is deprecated and will be removed in the next major version of pnpm. ` + + 'It keeps working until then, but it opts the installation out of the parallel package importer, making it slower.' + ) } } diff --git a/pnpm11/hooks/pnpmfile/test/__fixtures__/importPackage.js b/pnpm11/hooks/pnpmfile/test/__fixtures__/importPackage.js new file mode 100644 index 0000000000..bdeb8af185 --- /dev/null +++ b/pnpm11/hooks/pnpmfile/test/__fixtures__/importPackage.js @@ -0,0 +1,7 @@ +module.exports = { + hooks: {importPackage} +} + +async function importPackage() { + return 'copy' +} diff --git a/pnpm11/hooks/pnpmfile/test/importPackageDeprecation.test.ts b/pnpm11/hooks/pnpmfile/test/importPackageDeprecation.test.ts new file mode 100644 index 0000000000..a5f8f5b9d8 --- /dev/null +++ b/pnpm11/hooks/pnpmfile/test/importPackageDeprecation.test.ts @@ -0,0 +1,58 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +import { beforeEach, expect, jest, test } from '@jest/globals' + +jest.unstable_mockModule('@pnpm/logger', () => ({ + globalWarn: jest.fn(), + logger: () => ({ debug: jest.fn() }), +})) + +const { globalWarn } = await import('@pnpm/logger') +const { requireHooks } = await import('../lib/requireHooks.js') + +const testOnPosix = process.platform === 'win32' ? test.skip : test + +beforeEach(() => { + jest.mocked(globalWarn).mockClear() +}) + +test('requireHooks() warns that the importPackage hook is deprecated', async () => { + const pnpmfile = path.join(import.meta.dirname, '__fixtures__/importPackage.js') + const { hooks } = await requireHooks(import.meta.dirname, { pnpmfiles: [pnpmfile] }) + + expect(hooks.importPackage).toBeDefined() + expect(globalWarn).toHaveBeenCalledTimes(1) + const warning = jest.mocked(globalWarn).mock.calls[0][0] + expect(warning).toContain('"importPackage" hook') + expect(warning).toContain(pnpmfile) + expect(warning).toContain('deprecated') + expect(warning).toContain('will be removed in the next major version') + expect(warning).toContain('keeps working until then') + expect(warning).toContain('parallel package importer') +}) + +test('requireHooks() does not warn when no importPackage hook is defined', async () => { + const pnpmfile = path.join(import.meta.dirname, '__fixtures__/filterLog.js') + await requireHooks(import.meta.dirname, { pnpmfiles: [pnpmfile] }) + + expect(globalWarn).not.toHaveBeenCalled() +}) + +testOnPosix('requireHooks() strips control characters from the pnpmfile path in the warning', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pnpmfile-')) + const pnpmfile = path.join(dir, 'spoofed\u001b[2K\nnot-really-a-hook.pnpmfile.cjs') + fs.copyFileSync(path.join(import.meta.dirname, '__fixtures__/importPackage.js'), pnpmfile) + + try { + await requireHooks(dir, { pnpmfiles: [pnpmfile] }) + } finally { + fs.rmSync(dir, { recursive: true, force: true }) + } + + const warning = jest.mocked(globalWarn).mock.calls[0][0] + expect(warning).not.toContain('\u001b') + expect(warning).not.toContain('\n') + expect(warning).toContain('not-really-a-hook.pnpmfile.cjs') +})