From 78cf081f978a4e30b95f811bb3446e916bcea69b Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 23 Aug 2026 15:07:44 +0200 Subject: [PATCH] feat: deprecate the importPackage pnpmfile hook (#14105) A pnpmfile that defines `hooks.importPackage` now gets a warning saying the hook is deprecated and will be removed in the next major version. The hook lets a pnpmfile take over writing a package into node_modules. It is a poor fit for where both stacks are heading: - Defining it drops the installation off the worker-thread importer entirely (`store/controller/src/storeController/index.ts`), and `store/create-cafs-store/src/index.ts` shows it also bypasses `pkgImportMethod` -- including the `willBeBuilt -> clone-or-copy` rule, so a hook that hardlinks corrupts the store once the package is built. - Porting it to the Rust CLI would put a JS callback on the hottest phase of the install, behind the single sequential Node worker that serves pnpmfile hooks, carrying the whole filesMap per package. It would also hand user JS the invariants `import_indexed_dir` owns: exclusive-mkdir ownership claiming for shared slots, marker-based repair, and the macOS quarantine sweep. - A GitHub-wide code search finds no pnpmfile using it outside copies of the documentation page, except one dev-server tool that only needs to know where a package landed and to force an import method. The warning points at pnpm/pnpm#14101 so anyone who does depend on the hook can say so before it is removed. Related to pnpm/pnpm#14101. --- .changeset/deprecate-import-package-hook.md | 6 ++ pnpm11/hooks/pnpmfile/src/requireHooks.ts | 7 ++- .../test/__fixtures__/importPackage.js | 7 +++ .../test/importPackageDeprecation.test.ts | 58 +++++++++++++++++++ 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 .changeset/deprecate-import-package-hook.md create mode 100644 pnpm11/hooks/pnpmfile/test/__fixtures__/importPackage.js create mode 100644 pnpm11/hooks/pnpmfile/test/importPackageDeprecation.test.ts 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') +})