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.
This commit is contained in:
1 parent
9720df211a
commit
78cf081f97
4 files changed
+77
-1
No files matched your search
@@ -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).
|
||||
@@ -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.'
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
module.exports = {
|
||||
hooks: {importPackage}
|
||||
}
|
||||
|
||||
async function importPackage() {
|
||||
return 'copy'
|
||||
}
|
||||
@@ -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')
|
||||
})
|
||||
Reference in new issue
Block a user