From 3f0137077e9dc988f373c0e46edae34547e216c0 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 10 Sep 2022 19:34:14 +0300 Subject: [PATCH] fix(patch): allow to edit a package in any directory (#5331) close #5330 ref #5293 --- .changeset/fluffy-crews-join.md | 6 +++ .../plugin-commands-patching/src/patch.ts | 43 +++------------- .../src/patchCommit.ts | 9 ++-- .../src/writePackage.ts | 29 +++++++++++ .../test/patch.test.ts | 50 +++++++++++-------- 5 files changed, 77 insertions(+), 60 deletions(-) create mode 100644 .changeset/fluffy-crews-join.md create mode 100644 packages/plugin-commands-patching/src/writePackage.ts diff --git a/.changeset/fluffy-crews-join.md b/.changeset/fluffy-crews-join.md new file mode 100644 index 00000000000..f325c5c7ea6 --- /dev/null +++ b/.changeset/fluffy-crews-join.md @@ -0,0 +1,6 @@ +--- +"@pnpm/plugin-commands-patching": patch +"pnpm": patch +--- + +Fix `pnpm patch` using a custom `--edit-dir`. diff --git a/packages/plugin-commands-patching/src/patch.ts b/packages/plugin-commands-patching/src/patch.ts index 709b1ce2f0e..017af744346 100644 --- a/packages/plugin-commands-patching/src/patch.ts +++ b/packages/plugin-commands-patching/src/patch.ts @@ -1,18 +1,15 @@ 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 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' +import { writePackage } from './writePackage' export function rcOptionsTypes () { return pick([], allTypes) @@ -51,38 +48,10 @@ export type PatchCommandOptions = Pick 0) { + throw new PnpmError('PATCH_EDIT_DIR_EXISTS', `The target directory already exists: '${opts.editDir}'`) } - fs.mkdirSync(editDir, { recursive: true }) - return editDir + const editDir = opts.editDir ?? tempy.directory() + await writePackage(params[0], editDir, opts) + return `You can now edit the following folder: ${editDir}` } diff --git a/packages/plugin-commands-patching/src/patchCommit.ts b/packages/plugin-commands-patching/src/patchCommit.ts index 96fb78002c5..4c22b4102f0 100644 --- a/packages/plugin-commands-patching/src/patchCommit.ts +++ b/packages/plugin-commands-patching/src/patchCommit.ts @@ -9,6 +9,8 @@ import pick from 'ramda/src/pick' import execa from 'safe-execa' import escapeStringRegexp from 'escape-string-regexp' import renderHelp from 'render-help' +import tempy from 'tempy' +import { writePackage } from './writePackage' export const rcOptionsTypes = cliOptionsTypes @@ -29,13 +31,14 @@ export function help () { export async function handler (opts: install.InstallCommandOptions, params: string[]) { const userDir = params[0] - const srcDir = path.join(userDir, '../source') - const patchContent = await diffFolders(srcDir, userDir) const lockfileDir = opts.lockfileDir ?? opts.dir ?? process.cwd() const patchesDir = path.join(lockfileDir, 'patches') await fs.promises.mkdir(patchesDir, { recursive: true }) - const patchedPkgManifest = await readPackageJsonFromDir(srcDir) + const patchedPkgManifest = await readPackageJsonFromDir(userDir) const pkgNameAndVersion = `${patchedPkgManifest.name}@${patchedPkgManifest.version}` + const srcDir = tempy.directory() + await writePackage(pkgNameAndVersion, srcDir, opts) + const patchContent = await diffFolders(srcDir, userDir) const patchFileName = pkgNameAndVersion.replace('/', '__') await fs.promises.writeFile(path.join(patchesDir, `${patchFileName}.patch`), patchContent, 'utf8') let { manifest, writeProjectManifest } = await tryReadProjectManifest(lockfileDir) diff --git a/packages/plugin-commands-patching/src/writePackage.ts b/packages/plugin-commands-patching/src/writePackage.ts new file mode 100644 index 00000000000..21c3a8702ba --- /dev/null +++ b/packages/plugin-commands-patching/src/writePackage.ts @@ -0,0 +1,29 @@ +import { Config } from '@pnpm/config' +import { + createOrConnectStoreController, + CreateStoreControllerOptions, +} from '@pnpm/store-connection-manager' +import pickRegistryForPackage from '@pnpm/pick-registry-for-package' +import parseWantedDependency from '@pnpm/parse-wanted-dependency' + +export type WritePackageOptions = CreateStoreControllerOptions & Pick + +export async function writePackage (pkg: string, dest: string, opts: WritePackageOptions) { + const dep = parseWantedDependency(pkg) + const store = await createOrConnectStoreController({ + ...opts, + packageImportMethod: 'clone-or-copy', + }) + const pkgResponse = await store.ctrl.requestPackage(dep, { + downloadPriority: 1, + lockfileDir: opts.dir, + preferredVersions: {}, + projectDir: opts.dir, + registry: (dep.alias && pickRegistryForPackage(opts.registries, dep.alias)) ?? opts.registries.default, + }) + const filesResponse = await pkgResponse.files!() + await store.ctrl.importPackage(dest, { + filesResponse, + force: true, + }) +} diff --git a/packages/plugin-commands-patching/test/patch.test.ts b/packages/plugin-commands-patching/test/patch.test.ts index 1a233d968be..c6b6169166a 100644 --- a/packages/plugin-commands-patching/test/patch.test.ts +++ b/packages/plugin-commands-patching/test/patch.test.ts @@ -10,7 +10,6 @@ import { DEFAULT_OPTS } from './utils/index' describe('patch and commit', () => { let defaultPatchOption: patch.PatchCommandOptions - const tempySpy = jest.spyOn(tempy, 'directory') beforeEach(() => { prepare({ @@ -37,24 +36,23 @@ describe('patch and commit', () => { 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 patchDir = output.substring(output.indexOf(':') + 1).trim() const tempDir = os.tmpdir() // temp dir depends on the operating system (@see tempy) - // 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) + // store patch files in a temporary directory when not given editDir option + expect(patchDir).toContain(tempDir) + expect(fs.existsSync(patchDir)).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)') + expect(fs.readFileSync(path.join(patchDir, '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(patchDir, 'index.js'), '// test patching', 'utf8') + fs.unlinkSync(path.join(patchDir, 'license')) await patchCommit.handler({ ...DEFAULT_OPTS, dir: process.cwd(), - }, [userPatchDir]) + }, [patchDir]) const { manifest } = await readProjectManifest(process.cwd()) expect(manifest.pnpm?.patchedDependencies).toStrictEqual({ @@ -69,18 +67,30 @@ describe('patch and commit', () => { 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' + test('patch and commit with a custom edit dir', async () => { + const editDir = path.join(tempy.directory()) - const patchFn = async () => patch.handler({ ...defaultPatchOption, editDir }, ['is-positive@1.0.0']) - const output = await patchFn() - const userPatchDir = output.substring(output.indexOf(':') + 1).trim() + const output = await patch.handler({ ...defaultPatchOption, editDir }, ['is-positive@1.0.0']) + const patchDir = 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) + expect(patchDir).toBe(editDir) + expect(fs.existsSync(patchDir)).toBe(true) - // If editDir already exists, it should throw an error - await expect(patchFn()).rejects.toThrow(`The target directory already exists: '${editDir}'`) + fs.appendFileSync(path.join(patchDir, 'index.js'), '// test patching', 'utf8') + + await patchCommit.handler({ + ...DEFAULT_OPTS, + dir: process.cwd(), + }, [patchDir]) + + expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching') + }) + + test('patch throws an error if the edit-dir already exists and is not empty', async () => { + const editDir = tempy.directory() + fs.writeFileSync(path.join(editDir, 'test.txt'), '', 'utf8') + + await expect(() => patch.handler({ ...defaultPatchOption, editDir }, ['is-positive@1.0.0'])) + .rejects.toThrow(`The target directory already exists: '${editDir}'`) }) })