fix: npm should not run lifecycle events on pnpm publish (#3340)

close #3332

.changeset/fair-carpets-mix.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/exportable-manifest": patch
+---
+
+Remove publish lifecycle events from manifest to avoid npm running them.

packages/exportable-manifest/src/index.ts

@@ -21,8 +21,20 @@ const PUBLISH_CONFIG_WHITELIST = new Set([
   'umd:main',
 ])
 
+const PREPUBLISH_SCRIPTS = [
+  'prepublishOnly',
+  'prepack',
+  'prepare',
+  'postpack',
+  'publish',
+  'postpublish',
+]
+
 export default async function makePublishManifest (dir: string, originalManifest: ProjectManifest) {
-  const publishManifest = R.omit(['pnpm'], originalManifest)
+  const publishManifest: ProjectManifest = R.omit(['pnpm', 'scripts'], originalManifest)
+  if (originalManifest.scripts != null) {
+    publishManifest.scripts = R.omit(PREPUBLISH_SCRIPTS, originalManifest.scripts)
+  }
   for (const depsField of ['dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies']) {
     const deps = await makePublishDependencies(dir, originalManifest[depsField])
     if (deps != null) {

packages/exportable-manifest/test/index.test.ts

@@ -21,3 +21,27 @@ test('the pnpm options are removed', async () => {
     },
   })
 })
+
+test('publish lifecycle scripts are removed', async () => {
+  expect(await exportableManifest(process.cwd(), {
+    name: 'foo',
+    version: '1.0.0',
+    scripts: {
+      prepublishOnly: 'echo',
+      prepack: 'echo',
+      prepare: 'echo',
+      postpack: 'echo',
+      publish: 'echo',
+      postpublish: 'echo',
+      postinstall: 'echo',
+      test: 'echo',
+    },
+  })).toStrictEqual({
+    name: 'foo',
+    version: '1.0.0',
+    scripts: {
+      postinstall: 'echo',
+      test: 'echo',
+    },
+  })
+})

packages/plugin-commands-publishing/src/publish.ts

@@ -153,23 +153,23 @@ Do you want to continue?`,
     unsafePerm: true, // when running scripts explicitly, assume that they're trusted.
   })
   let _status!: number
+  const { manifest } = await readProjectManifest(dir, opts)
+  // Unfortunately, we cannot support postpack at the moment
+  if (!opts.ignoreScripts) {
+    await _runScriptsIfPresent([
+      'prepublish',
+      'prepare',
+      'prepublishOnly',
+      'prepack',
+    ], manifest)
+  }
   await fakeRegularManifest(
     {
       dir,
       engineStrict: opts.engineStrict,
       workspaceDir: opts.workspaceDir ?? dir,
     },
-    async (publishManifest) => {
-      // Unfortunately, we cannot support postpack at the moment
-      if (!opts.ignoreScripts) {
-        await _runScriptsIfPresent([
-          'prepublish',
-          'prepare',
-          'prepublishOnly',
-          'prepack',
-        ], publishManifest)
-      }
-
+    async () => {
       const args = opts.argv.original.slice(1)
       const index = args.indexOf('--publish-branch')
       if (index !== -1) {
@@ -183,18 +183,18 @@ Do you want to continue?`,
       }
 
       const { status } = runNpm(opts.npmPath, ['publish', '--ignore-scripts', ...args])
-      if (!opts.ignoreScripts) {
-        await _runScriptsIfPresent([
-          'publish',
-          'postpublish',
-        ], publishManifest)
-      }
       _status = status!
     }
   )
   if (_status !== 0) {
     process.exit(_status)
   }
+  if (!opts.ignoreScripts) {
+    await _runScriptsIfPresent([
+      'publish',
+      'postpublish',
+    ], manifest)
+  }
 }
 
 async function runScriptsIfPresent (
@@ -217,7 +217,7 @@ export async function fakeRegularManifest (
     dir: string
     workspaceDir: string
   },
-  fn: (publishManifest: ProjectManifest) => Promise<void>
+  fn: () => Promise<void>
 ) {
   // If a workspace package has no License of its own,
   // license files from the root of the workspace are used
@@ -232,7 +232,7 @@ export async function fakeRegularManifest (
     await rimraf(path.join(opts.dir, fileName))
     await writeJsonFile(path.join(opts.dir, 'package.json'), publishManifest)
   }
-  await fn(publishManifest)
+  await fn()
   if (replaceManifest) {
     await rimraf(path.join(opts.dir, 'package.json'))
     await writeProjectManifest(manifest, true)

packages/plugin-commands-publishing/test/recursivePublish.ts

@@ -1,4 +1,5 @@
 import { promises as fs } from 'fs'
+import path from 'path'
 import { readProjects } from '@pnpm/filter-workspace-packages'
 import { streamParser } from '@pnpm/logger'
 import { publish } from '@pnpm/plugin-commands-publishing'
@@ -10,12 +11,11 @@ import crossSpawn from 'cross-spawn'
 import loadJsonFile from 'load-json-file'
 import { DEFAULT_OPTS } from './utils'
 
-const CREDENTIALS = [
-  `--registry=http://localhost:${REGISTRY_MOCK_PORT}/`,
-  `--//localhost:${REGISTRY_MOCK_PORT}/:username=username`,
-  `--//localhost:${REGISTRY_MOCK_PORT}/:_password=${Buffer.from('password').toString('base64')}`,
-  `--//localhost:${REGISTRY_MOCK_PORT}/:email=foo@bar.net`,
-].join('\n')
+const CREDENTIALS = `\
+registry=http://localhost:${REGISTRY_MOCK_PORT}/
+//localhost:${REGISTRY_MOCK_PORT}/:username=username
+//localhost:${REGISTRY_MOCK_PORT}/:_password=${Buffer.from('password').toString('base64')}
+//localhost:${REGISTRY_MOCK_PORT}/:email=foo@bar.net`
 
 test('recursive publish', async () => {
   const pkg1 = {
@@ -86,6 +86,7 @@ test('recursive publish', async () => {
     expect(status).toBe(1)
   }
 
+  process.env.npm_config_userconfig = path.join('.npmrc')
   await publish.handler({
     ...DEFAULT_OPTS,
     ...await readProjects(process.cwd(), []),

packages/pnpm/test/cli.ts

@@ -1,6 +1,6 @@
 import { createReadStream, promises as fs } from 'fs'
 import path from 'path'
-import prepare from '@pnpm/prepare'
+import prepare, { prepareEmpty } from '@pnpm/prepare'
 import rimraf from '@zkochan/rimraf'
 import execa from 'execa'
 import loadJsonFile from 'load-json-file'
@@ -122,6 +122,8 @@ test('adding new dep does not fail if node_modules was created with --public-hoi
 })
 
 test('pnpx works', () => {
+  prepareEmpty()
+
   const result = execPnpxSync(['--yes', 'hello-world-js-bin'])
 
   expect(result.status).toBe(0)

packages/pnpm/test/install/misc.ts

@@ -1,4 +1,4 @@
-import { promises as fs, readFileSync } from 'fs'
+import { promises as fs } from 'fs'
 import path from 'path'
 import { WANTED_LOCKFILE } from '@pnpm/constants'
 import { Lockfile } from '@pnpm/lockfile-types'
@@ -12,7 +12,6 @@ import rimraf from '@zkochan/rimraf'
 import isWindows from 'is-windows'
 import loadJsonFile from 'load-json-file'
 import exists from 'path-exists'
-import semver from 'semver'
 import crossSpawn from 'cross-spawn'
 import {
   execPnpm,
@@ -166,29 +165,6 @@ test("don't fail on case insensitive filesystems when package has 2 files with s
   }
 })
 
-test('lockfile compatibility', async () => {
-  if (semver.satisfies(process.version, '4')) {
-    console.log("don't run on Node.js 4")
-    return
-  }
-  prepare({ dependencies: { rimraf: '*' } })
-
-  await execPnpm(['install', 'rimraf@2.5.1'])
-
-  return new Promise<void>((resolve, reject) => {
-    const proc = crossSpawn.spawn('npm', ['shrinkwrap'])
-
-    proc.on('error', reject)
-
-    proc.on('close', (code: number) => {
-      if (code > 0) return reject(new Error(`Exit code ${code}`))
-      const wrap = JSON.parse(readFileSync('npm-shrinkwrap.json', 'utf-8'))
-      expect(wrap.dependencies.rimraf.version).toBe('2.5.1')
-      resolve()
-    })
-  })
-})
-
 test('top-level packages should find the plugins they use', async () => {
   prepare({
     scripts: {