fix(patching): reuse patch without version (#8936)

close #8919
pnpm · Jan 4, 2025 · 0f35416 · 0f35416
1 parent 9591a18
commit 0f35416
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 8 deletions.
diff --git a/.changeset/popular-hats-confess.md b/.changeset/popular-hats-confess.md
@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-patching": patch
+"pnpm": patch
+---
+
+Fix a bug in which `pnpm patch` is unable to bring back old patch without specifying `@version` suffix [#8919](https://github.com/pnpm/pnpm/issues/8919).
diff --git a/patching/plugin-commands-patching/src/patch.ts b/patching/plugin-commands-patching/src/patch.ts
@@ -12,10 +12,9 @@ import renderHelp from 'render-help'
 import chalk from 'chalk'
 import terminalLink from 'terminal-link'
 import { PnpmError } from '@pnpm/error'
-import { type ParseWantedDependencyResult } from '@pnpm/parse-wanted-dependency'
 import { writePackage } from './writePackage'
 import { getEditDirPath } from './getEditDirPath'
-import { getPatchedDependency } from './getPatchedDependency'
+import { type GetPatchedDependencyResult, getPatchedDependency } from './getPatchedDependency'
 import { writeEditDirState } from './stateFile'
 import { tryReadProjectManifest } from '@pnpm/read-project-manifest'
 import isWindows from 'is-windows'
@@ -140,22 +139,26 @@ To commit your changes, run:
 function tryPatchWithExistingPatchFile (
   {
     allowFailure,
-    patchedDep,
+    patchedDep: { applyToAll, alias, pref },
     patchedDir,
     patchedDependencies,
     lockfileDir,
   }: {
     allowFailure: boolean
-    patchedDep: ParseWantedDependencyResult
+    patchedDep: GetPatchedDependencyResult
     patchedDir: string
     patchedDependencies: Record<string, string>
     lockfileDir: string
   }
 ): void {
-  if (!patchedDep.alias || !patchedDep.pref) {
-    return
+  if (!alias) return
+  let existingPatchFile: string | undefined
+  if (pref) {
+    existingPatchFile = patchedDependencies[`${alias}@${pref}`]
+  }
+  if (!existingPatchFile && applyToAll) {
+    existingPatchFile = patchedDependencies[alias]
   }
-  const existingPatchFile = patchedDependencies[`${patchedDep.alias}@${patchedDep.pref}`]
   if (!existingPatchFile) {
     return
   }

diff --git a/patching/plugin-commands-patching/test/patch.test.ts b/patching/plugin-commands-patching/test/patch.test.ts
@@ -429,7 +429,7 @@ describe('patch and commit', () => {
     expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching')
   })
 
-  test('should reuse existing patch file by default', async () => {
+  test('should reuse existing patch file by default (with version suffix)', async () => {
     let output = await patch.handler(defaultPatchOption, ['is-positive@1.0.0'])
     let patchDir = getPatchDirFromPatchOutput(output)
 
@@ -462,6 +462,39 @@ describe('patch and commit', () => {
     expect(fs.readFileSync(path.join(patchDir, 'index.js'), 'utf8')).toContain('// test patching')
   })
 
+  test('should reuse existing patch file by default (without version suffix)', async () => {
+    let output = await patch.handler(defaultPatchOption, ['is-positive'])
+    let patchDir = getPatchDirFromPatchOutput(output)
+
+    fs.appendFileSync(path.join(patchDir, 'index.js'), '// test patching', 'utf8')
+    fs.unlinkSync(path.join(patchDir, 'license'))
+
+    await patchCommit.handler({
+      ...DEFAULT_OPTS,
+      cacheDir,
+      dir: process.cwd(),
+      rootProjectManifestDir: process.cwd(),
+      frozenLockfile: false,
+      fixLockfile: true,
+      storeDir,
+    }, [patchDir])
+
+    const { manifest } = await readProjectManifest(process.cwd())
+    expect(manifest.pnpm?.patchedDependencies).toStrictEqual({
+      'is-positive': 'patches/is-positive.patch',
+    })
+    expect(fs.existsSync('patches/is-positive.patch')).toBe(true)
+
+    // re-patch
+    fs.rmSync(patchDir, { recursive: true })
+    output = await patch.handler({ ...defaultPatchOption, rootProjectManifest: manifest }, ['is-positive'])
+    patchDir = getPatchDirFromPatchOutput(output)
+
+    expect(fs.existsSync(patchDir)).toBe(true)
+    expect(fs.existsSync(path.join(patchDir, 'license'))).toBe(false)
+    expect(fs.readFileSync(path.join(patchDir, 'index.js'), 'utf8')).toContain('// test patching')
+  })
+
   test('if the patch file is not existed when patching, should throw an error', async () => {
     const { writeProjectManifest, manifest } = await readProjectManifest(process.cwd())
     await writeProjectManifest({