From de561a55fa6b8cb3cb7a6f4d72f94445bda069f7 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 1 Mar 2026 17:00:33 +0100 Subject: [PATCH] fix: prefer .pnpmfile.mjs by default (#10683) * fix: prefer .pnpmfile.mjs by default * fix: handle ERR_MODULE_NOT_FOUND for missing optional .pnpmfile.mjs Node's dynamic import() throws ERR_MODULE_NOT_FOUND (not MODULE_NOT_FOUND like require()) when a file doesn't exist. This caused a hard error when tryLoadDefaultPnpmfile was enabled and .pnpmfile.mjs was absent. * fix: load only .pnpmfile.mjs when it exists, not both .mjs and .cjs When both .pnpmfile.mjs and .pnpmfile.cjs exist, only the .mjs file is now loaded. Previously both were loaded and their hooks combined. Also adds .mjs support for config dependency plugins. --- .changeset/warm-pnpmfiles-glow.md | 6 ++++ cli/cli-utils/src/getConfig.ts | 10 ++++-- cli/cli-utils/src/index.ts | 2 +- cli/cli-utils/test/getConfig.test.ts | 34 ++++++++++++++++++- hooks/pnpmfile/src/requireHooks.ts | 29 ++++++++++++---- hooks/pnpmfile/src/requirePnpmfile.ts | 2 +- .../__fixtures__/default-both/.pnpmfile.cjs | 8 +++++ .../__fixtures__/default-both/.pnpmfile.mjs | 6 ++++ .../__fixtures__/default-esm/.pnpmfile.mjs | 3 ++ hooks/pnpmfile/test/index.ts | 13 +++++++ 10 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 .changeset/warm-pnpmfiles-glow.md create mode 100644 hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.cjs create mode 100644 hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.mjs create mode 100644 hooks/pnpmfile/test/__fixtures__/default-esm/.pnpmfile.mjs diff --git a/.changeset/warm-pnpmfiles-glow.md b/.changeset/warm-pnpmfiles-glow.md new file mode 100644 index 0000000000..d3afd153f6 --- /dev/null +++ b/.changeset/warm-pnpmfiles-glow.md @@ -0,0 +1,6 @@ +--- +"@pnpm/pnpmfile": minor +"pnpm": minor +--- + +Support `.pnpmfile.mjs` as the default pnpmfile. When `.pnpmfile.mjs` exists, it takes priority over `.pnpmfile.cjs` and only one is loaded. diff --git a/cli/cli-utils/src/getConfig.ts b/cli/cli-utils/src/getConfig.ts index 80e511a739..00f60b14b4 100644 --- a/cli/cli-utils/src/getConfig.ts +++ b/cli/cli-utils/src/getConfig.ts @@ -1,3 +1,4 @@ +import fs from 'fs' import path from 'path' import { packageManager } from '@pnpm/cli-meta' import { getConfig as _getConfig, type CliOptions, type Config } from '@pnpm/config' @@ -72,10 +73,15 @@ export async function getConfig ( return config } -function * calcPnpmfilePathsOfPluginDeps (configModulesDir: string, configDependencies: ConfigDependencies): Generator { +export function * calcPnpmfilePathsOfPluginDeps (configModulesDir: string, configDependencies: ConfigDependencies): Generator { for (const configDepName of Object.keys(configDependencies).sort(lexCompare)) { if (isPluginName(configDepName)) { - yield path.join(configModulesDir, configDepName, 'pnpmfile.cjs') + const mjsPath = path.join(configModulesDir, configDepName, 'pnpmfile.mjs') + if (fs.existsSync(mjsPath)) { + yield mjsPath + } else { + yield path.join(configModulesDir, configDepName, 'pnpmfile.cjs') + } } } } diff --git a/cli/cli-utils/src/index.ts b/cli/cli-utils/src/index.ts index 2472b03775..86b4f50442 100644 --- a/cli/cli-utils/src/index.ts +++ b/cli/cli-utils/src/index.ts @@ -1,6 +1,6 @@ import { packageManager } from '@pnpm/cli-meta' -export { getConfig } from './getConfig.js' +export { calcPnpmfilePathsOfPluginDeps, getConfig } from './getConfig.js' export * from './packageIsInstallable.js' export * from './readDepNameCompletions.js' export * from './readProjectManifest.js' diff --git a/cli/cli-utils/test/getConfig.test.ts b/cli/cli-utils/test/getConfig.test.ts index 6e13af1bd4..ab75a5f7ed 100644 --- a/cli/cli-utils/test/getConfig.test.ts +++ b/cli/cli-utils/test/getConfig.test.ts @@ -1,6 +1,7 @@ /// import fs from 'fs' -import { getConfig } from '@pnpm/cli-utils' +import path from 'path' +import { calcPnpmfilePathsOfPluginDeps, getConfig } from '@pnpm/cli-utils' import { prepare } from '@pnpm/prepare' import { jest } from '@jest/globals' @@ -28,6 +29,37 @@ test('console a warning when the .npmrc has an env variable that does not exist' expect(console.warn).toHaveBeenCalledWith(expect.stringContaining('Failed to replace env in config: ${ENV_VAR_123}')) }) +describe('calcPnpmfilePathsOfPluginDeps', () => { + test('yields pnpmfile.mjs when it exists', () => { + const tmpDir = fs.mkdtempSync(path.join(import.meta.dirname, '.tmp-')) + try { + const pluginDir = path.join(tmpDir, 'pnpm-plugin-foo') + fs.mkdirSync(pluginDir, { recursive: true }) + fs.writeFileSync(path.join(pluginDir, 'pnpmfile.mjs'), '') + fs.writeFileSync(path.join(pluginDir, 'pnpmfile.cjs'), '') + + const paths = [...calcPnpmfilePathsOfPluginDeps(tmpDir, { 'pnpm-plugin-foo': '1.0.0' })] + expect(paths).toEqual([path.join(pluginDir, 'pnpmfile.mjs')]) + } finally { + fs.rmSync(tmpDir, { recursive: true }) + } + }) + + test('falls back to pnpmfile.cjs when pnpmfile.mjs does not exist', () => { + const tmpDir = fs.mkdtempSync(path.join(import.meta.dirname, '.tmp-')) + try { + const pluginDir = path.join(tmpDir, 'pnpm-plugin-foo') + fs.mkdirSync(pluginDir, { recursive: true }) + fs.writeFileSync(path.join(pluginDir, 'pnpmfile.cjs'), '') + + const paths = [...calcPnpmfilePathsOfPluginDeps(tmpDir, { 'pnpm-plugin-foo': '1.0.0' })] + expect(paths).toEqual([path.join(pluginDir, 'pnpmfile.cjs')]) + } finally { + fs.rmSync(tmpDir, { recursive: true }) + } + }) +}) + test('hoist: false removes hoistPattern', async () => { prepare() diff --git a/hooks/pnpmfile/src/requireHooks.ts b/hooks/pnpmfile/src/requireHooks.ts index 543de7d0c0..ddac8b4015 100644 --- a/hooks/pnpmfile/src/requireHooks.ts +++ b/hooks/pnpmfile/src/requireHooks.ts @@ -65,12 +65,29 @@ export async function requireHooks ( includeInChecksum: false, }) } + const entries: PnpmfileEntryLoaded[] = [] + const loadedFiles: string[] = [] if (opts.tryLoadDefaultPnpmfile) { - pnpmfiles.push({ - path: '.pnpmfile.cjs', - includeInChecksum: true, - optional: true, - }) + // Prefer .pnpmfile.mjs over .pnpmfile.cjs. Only load one. + const mjsPath = pathAbsolute('.pnpmfile.mjs', prefix) + const mjsResult = await requirePnpmfile(mjsPath, prefix) + if (mjsResult != null) { + loadedFiles.push(mjsPath) + entries.push({ + file: mjsPath, + includeInChecksum: true, + hooks: mjsResult.pnpmfileModule?.hooks, + finders: mjsResult.pnpmfileModule?.finders, + resolvers: mjsResult.pnpmfileModule?.resolvers, + fetchers: mjsResult.pnpmfileModule?.fetchers, + }) + } else { + pnpmfiles.push({ + path: '.pnpmfile.cjs', + includeInChecksum: true, + optional: true, + }) + } } if (opts.pnpmfiles) { for (const pnpmfile of opts.pnpmfiles) { @@ -80,8 +97,6 @@ export async function requireHooks ( }) } } - const entries: PnpmfileEntryLoaded[] = [] - const loadedFiles: string[] = [] await Promise.all(pnpmfiles.map(async ({ path, includeInChecksum, optional }) => { const file = pathAbsolute(path, prefix) if (!loadedFiles.includes(file)) { diff --git a/hooks/pnpmfile/src/requirePnpmfile.ts b/hooks/pnpmfile/src/requirePnpmfile.ts index 0e8752f5bf..19c7dd0d48 100644 --- a/hooks/pnpmfile/src/requirePnpmfile.ts +++ b/hooks/pnpmfile/src/requirePnpmfile.ts @@ -95,7 +95,7 @@ export async function requirePnpmfile (pnpmFilePath: string, prefix: string): Pr } assert(util.types.isNativeError(err)) if ( - !('code' in err && err.code === 'MODULE_NOT_FOUND') || + !('code' in err && (err.code === 'MODULE_NOT_FOUND' || err.code === 'ERR_MODULE_NOT_FOUND')) || pnpmFileExistsSync(pnpmFilePath) ) { throw new PnpmFileFailError(pnpmFilePath, err) diff --git a/hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.cjs b/hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.cjs new file mode 100644 index 0000000000..e0d3857362 --- /dev/null +++ b/hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.cjs @@ -0,0 +1,8 @@ +module.exports = { + hooks: { + readPackage: (pkg) => { + pkg._fromCjs = true + return pkg + }, + } +} diff --git a/hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.mjs b/hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.mjs new file mode 100644 index 0000000000..c482b7e2a3 --- /dev/null +++ b/hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.mjs @@ -0,0 +1,6 @@ +export const hooks = { + readPackage: (pkg) => { + pkg._fromMjs = true + return pkg + }, +} diff --git a/hooks/pnpmfile/test/__fixtures__/default-esm/.pnpmfile.mjs b/hooks/pnpmfile/test/__fixtures__/default-esm/.pnpmfile.mjs new file mode 100644 index 0000000000..d96d9cc3e1 --- /dev/null +++ b/hooks/pnpmfile/test/__fixtures__/default-esm/.pnpmfile.mjs @@ -0,0 +1,3 @@ +export const hooks = { + readPackage: (pkg) => pkg, +} diff --git a/hooks/pnpmfile/test/index.ts b/hooks/pnpmfile/test/index.ts index 9d2791b9ad..df92d8d095 100644 --- a/hooks/pnpmfile/test/index.ts +++ b/hooks/pnpmfile/test/index.ts @@ -59,6 +59,19 @@ test('loading the default pnpmfile if tryLoadDefaultPnpmfile is set to true', as expect(hooks.readPackage?.length).toBe(1) }) +test('loading the default .pnpmfile.mjs if tryLoadDefaultPnpmfile is set to true', async () => { + const { hooks } = await requireHooks(path.join(import.meta.dirname, '__fixtures__/default-esm'), { tryLoadDefaultPnpmfile: true }) + expect(hooks.readPackage?.length).toBe(1) +}) + +test('.pnpmfile.mjs takes priority over .pnpmfile.cjs when both exist', async () => { + const { hooks } = await requireHooks(path.join(import.meta.dirname, '__fixtures__/default-both'), { tryLoadDefaultPnpmfile: true }) + expect(hooks.readPackage?.length).toBe(1) + const pkg: any = await hooks.readPackage![0]({ name: 'test', version: '1.0.0' }) // eslint-disable-line + expect(pkg._fromMjs).toBe(true) + expect(pkg._fromCjs).toBeUndefined() +}) + test('calculatePnpmfileChecksum is undefined when pnpmfile does not exist', async () => { const { hooks } = await requireHooks(import.meta.dirname, {}) expect(hooks.calculatePnpmfileChecksum).toBeUndefined()