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

@@ -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.

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<string> {
+export function * calcPnpmfilePathsOfPluginDeps (configModulesDir: string, configDependencies: ConfigDependencies): Generator<string> {
   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')
+      }
     }
   }
 }

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'

cli/cli-utils/test/getConfig.test.ts

@@ -1,6 +1,7 @@
 /// <reference path="../../../__typings__/index.d.ts"/>
 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()
 

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)) {

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)

hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.cjs

@@ -0,0 +1,8 @@
+module.exports = {
+  hooks: {
+    readPackage: (pkg) => {
+      pkg._fromCjs = true
+      return pkg
+    },
+  }
+}

hooks/pnpmfile/test/__fixtures__/default-both/.pnpmfile.mjs

@@ -0,0 +1,6 @@
+export const hooks = {
+  readPackage: (pkg) => {
+    pkg._fromMjs = true
+    return pkg
+  },
+}

hooks/pnpmfile/test/__fixtures__/default-esm/.pnpmfile.mjs

@@ -0,0 +1,3 @@
+export const hooks = {
+  readPackage: (pkg) => pkg,
+}

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()