feat(plugin-command-patching): add path option to patch command (#5304)

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/funny-buses-own.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-patching": minor
+"pnpm": minor
+---
+
+`pnpm patch`: edit the patched package in a directory specified by the `--edit-dir` option. E.g., `pnpm patch express@3.1.0 --edit-dir=/home/xxx/src/patched-express`

packages/plugin-commands-patching/package.json

@@ -42,6 +42,7 @@
   "dependencies": {
     "@pnpm/cli-utils": "workspace:*",
     "@pnpm/config": "workspace:*",
+    "@pnpm/error": "workspace:*",
     "@pnpm/parse-wanted-dependency": "workspace:*",
     "@pnpm/pick-registry-for-package": "workspace:*",
     "@pnpm/plugin-commands-installation": "workspace:*",

packages/plugin-commands-patching/src/patch.ts

@@ -1,18 +1,29 @@
+import fs from 'fs'
 import path from 'path'
 import { docsUrl } from '@pnpm/cli-utils'
 import { Config, types as allTypes } from '@pnpm/config'
 import { LogBase } from '@pnpm/logger'
-import { createOrConnectStoreController, CreateStoreControllerOptions } from '@pnpm/store-connection-manager'
+import {
+  createOrConnectStoreController,
+  CreateStoreControllerOptions,
+} from '@pnpm/store-connection-manager'
 import parseWantedDependency from '@pnpm/parse-wanted-dependency'
 import pick from 'ramda/src/pick'
 import pickRegistryForPackage from '@pnpm/pick-registry-for-package'
 import renderHelp from 'render-help'
 import tempy from 'tempy'
+import PnpmError from '@pnpm/error'
 
-export const rcOptionsTypes = cliOptionsTypes
+export function rcOptionsTypes () {
+  return pick([], allTypes)
+}
 
 export function cliOptionsTypes () {
-  return pick([], allTypes)
+  return { ...rcOptionsTypes(), 'edit-dir': String }
+}
+
+export const shorthands = {
+  d: '--edit-dir',
 }
 
 export const commandNames = ['patch']
@@ -20,13 +31,22 @@ export const commandNames = ['patch']
 export function help () {
   return renderHelp({
     description: 'Prepare a package for patching',
-    descriptionLists: [],
+    descriptionLists: [{
+      title: 'Options',
+      list: [
+        {
+          description: 'The package that needs to be modified will be extracted to this directory',
+          name: '--edit-dir',
+        },
+      ],
+    }],
     url: docsUrl('patch'),
-    usages: ['pnpm patch'],
+    usages: ['pnpm patch <pkg name>@<version>'],
   })
 }
 
 export type PatchCommandOptions = Pick<Config, 'dir' | 'registries' | 'tag' | 'storeDir'> & CreateStoreControllerOptions & {
+  editDir?: string
   reporter?: (logObj: LogBase) => void
 }
 
@@ -45,7 +65,7 @@ export async function handler (opts: PatchCommandOptions, params: string[]) {
   })
   const filesResponse = await pkgResponse.files!()
   const tempDir = tempy.directory()
-  const userChangesDir = path.join(tempDir, 'user')
+  const userChangesDir = opts.editDir ? createPackageDirectory(opts.editDir) : path.join(tempDir, 'user')
   await Promise.all([
     store.ctrl.importPackage(path.join(tempDir, 'source'), {
       filesResponse,
@@ -58,3 +78,11 @@ export async function handler (opts: PatchCommandOptions, params: string[]) {
   ])
   return `You can now edit the following folder: ${userChangesDir}`
 }
+
+function createPackageDirectory (editDir: string) {
+  if (fs.existsSync(editDir)) {
+    throw new PnpmError('PATCH_EDIT_DIR_EXISTS', `The target directory already exists: '${editDir}'`)
+  }
+  fs.mkdirSync(editDir, { recursive: true })
+  return editDir
+}

packages/plugin-commands-patching/test/patch.test.ts

@@ -1,54 +1,86 @@
 import fs from 'fs'
+import os from 'os'
 import path from 'path'
 import prepare from '@pnpm/prepare'
+import tempy from 'tempy'
 import { patch, patchCommit } from '@pnpm/plugin-commands-patching'
 import readProjectManifest from '@pnpm/read-project-manifest'
 import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
 import { DEFAULT_OPTS } from './utils/index'
 
-test('patch and commit', async () => {
-  prepare({
-    dependencies: {
-      'is-positive': '1.0.0',
-    },
+describe('patch and commit', () => {
+  let defaultPatchOption: patch.PatchCommandOptions
+  const tempySpy = jest.spyOn(tempy, 'directory')
+
+  beforeEach(() => {
+    prepare({
+      dependencies: {
+        'is-positive': '1.0.0',
+      },
+    })
+
+    const cacheDir = path.resolve('cache')
+    const storeDir = path.resolve('store')
+
+    defaultPatchOption = {
+      cacheDir,
+      dir: process.cwd(),
+      pnpmHomeDir: '',
+      rawConfig: {
+        registry: `http://localhost:${REGISTRY_MOCK_PORT}/`,
+      },
+      registries: { default: `http://localhost:${REGISTRY_MOCK_PORT}/` },
+      storeDir,
+      userConfig: {},
+    }
   })
-  const cacheDir = path.resolve('cache')
-  const storeDir = path.resolve('store')
 
-  const output = await patch.handler({
-    cacheDir,
-    dir: process.cwd(),
-    pnpmHomeDir: '',
-    rawConfig: {
-      registry: `http://localhost:${REGISTRY_MOCK_PORT}/`,
-    },
-    registries: { default: `http://localhost:${REGISTRY_MOCK_PORT}/` },
-    storeDir,
-    userConfig: {},
-  }, ['is-positive@1.0.0'])
+  test('patch and commit', async () => {
+    const output = await patch.handler(defaultPatchOption, ['is-positive@1.0.0'])
+    const userPatchDir = output.substring(output.indexOf(':') + 1).trim()
+    const tempDir = os.tmpdir() // temp dir depends on the operating system (@see tempy)
 
-  const userPatchDir = output.substring(output.indexOf(':') + 1).trim()
+    // store patch files(user, source) in temporary directory when not given editDir option
+    expect(userPatchDir).toContain(tempDir)
+    expect(fs.existsSync(userPatchDir)).toBe(true)
+    expect(fs.existsSync(userPatchDir.replace('/user', '/source'))).toBe(true)
 
-  // sanity check to ensure that the license file contains the expected string
-  expect(fs.readFileSync(path.join(userPatchDir, 'license'), 'utf8')).toContain('The MIT License (MIT)')
+    // sanity check to ensure that the license file contains the expected string
+    expect(fs.readFileSync(path.join(userPatchDir, 'license'), 'utf8')).toContain('The MIT License (MIT)')
 
-  fs.appendFileSync(path.join(userPatchDir, 'index.js'), '// test patching', 'utf8')
-  fs.unlinkSync(path.join(userPatchDir, 'license'))
+    fs.appendFileSync(path.join(userPatchDir, 'index.js'), '// test patching', 'utf8')
+    fs.unlinkSync(path.join(userPatchDir, 'license'))
 
-  await patchCommit.handler({
-    ...DEFAULT_OPTS,
-    dir: process.cwd(),
-  }, [userPatchDir])
+    await patchCommit.handler({
+      ...DEFAULT_OPTS,
+      dir: process.cwd(),
+    }, [userPatchDir])
 
-  const { manifest } = await readProjectManifest(process.cwd())
-  expect(manifest.pnpm?.patchedDependencies).toStrictEqual({
-    'is-positive@1.0.0': 'patches/is-positive@1.0.0.patch',
+    const { manifest } = await readProjectManifest(process.cwd())
+    expect(manifest.pnpm?.patchedDependencies).toStrictEqual({
+      'is-positive@1.0.0': 'patches/is-positive@1.0.0.patch',
+    })
+    const patchContent = fs.readFileSync('patches/is-positive@1.0.0.patch', 'utf8')
+    expect(patchContent).toContain('diff --git')
+    expect(patchContent).toContain('// test patching')
+    expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching')
+
+    expect(patchContent).not.toContain('The MIT License (MIT)')
+    expect(fs.existsSync('node_modules/is-positive/license')).toBe(false)
   })
-  const patchContent = fs.readFileSync('patches/is-positive@1.0.0.patch', 'utf8')
-  expect(patchContent).toContain('diff --git')
-  expect(patchContent).toContain('// test patching')
-  expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching')
 
-  expect(patchContent).not.toContain('The MIT License (MIT)')
-  expect(fs.existsSync('node_modules/is-positive/license')).toBe(false)
+  test('store source files in temporary directory and user files in user directory, when given editDir option', async () => {
+    const editDir = 'test/user/is-positive'
+
+    const patchFn = async () => patch.handler({ ...defaultPatchOption, editDir }, ['is-positive@1.0.0'])
+    const output = await patchFn()
+    const userPatchDir = output.substring(output.indexOf(':') + 1).trim()
+
+    expect(userPatchDir).toBe(editDir)
+    expect(fs.existsSync(userPatchDir)).toBe(true)
+    expect(fs.existsSync(path.join(tempySpy.mock.results[0].value, '/source'))).toBe(true)
+
+    // If editDir already exists, it should throw an error
+    await expect(patchFn()).rejects.toThrow(`The target directory already exists: '${editDir}'`)
+  })
 })

packages/plugin-commands-patching/tsconfig.json

@@ -18,6 +18,9 @@
     {
       "path": "../config"
     },
+    {
+      "path": "../error"
+    },
     {
       "path": "../parse-wanted-dependency"
     },

pnpm-lock.yaml

@@ -3545,6 +3545,9 @@ importers:
       '@pnpm/config':
         specifier: workspace:*
         version: link:../config
+      '@pnpm/error':
+        specifier: workspace:*
+        version: link:../error
       '@pnpm/parse-wanted-dependency':
         specifier: workspace:*
         version: link:../parse-wanted-dependency
@@ -5597,7 +5600,7 @@ packages:
     engines: {node: '>=6.0.0'}
     hasBin: true
     dependencies:
-      '@babel/types': 7.17.10
+      '@babel/types': 7.18.13
     dev: true
 
   /@babel/parser/7.18.13_@babel+types@7.18.13: