From 5fea3d100fd2e47e8feeed3c280db051947cf311 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 10 Sep 2022 03:50:58 +0300 Subject: [PATCH 1/3] fix(patch): allow to edit a package in any directory 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 | 2 +- 5 files changed, 48 insertions(+), 41 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..87fb4261e22 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 + +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..4aaf33b9e86 100644 --- a/packages/plugin-commands-patching/test/patch.test.ts +++ b/packages/plugin-commands-patching/test/patch.test.ts @@ -78,7 +78,7 @@ describe('patch and commit', () => { expect(userPatchDir).toBe(editDir) expect(fs.existsSync(userPatchDir)).toBe(true) - expect(fs.existsSync(path.join(tempySpy.mock.results[0].value, '/source'))).toBe(true) + expect(fs.existsSync(tempySpy.mock.results[0].value)).toBe(true) // If editDir already exists, it should throw an error await expect(patchFn()).rejects.toThrow(`The target directory already exists: '${editDir}'`) From 28743bd6a91dbbbb8ac0850adbda71e78d3c517d Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 10 Sep 2022 16:44:51 +0300 Subject: [PATCH 2/3] refactor: test --- .../plugin-commands-patching/src/patch.ts | 2 +- .../test/patch.test.ts | 40 ++++++++++--------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/packages/plugin-commands-patching/src/patch.ts b/packages/plugin-commands-patching/src/patch.ts index 87fb4261e22..017af744346 100644 --- a/packages/plugin-commands-patching/src/patch.ts +++ b/packages/plugin-commands-patching/src/patch.ts @@ -48,7 +48,7 @@ export type PatchCommandOptions = Pick 0) { throw new PnpmError('PATCH_EDIT_DIR_EXISTS', `The target directory already exists: '${opts.editDir}'`) } const editDir = opts.editDir ?? tempy.directory() diff --git a/packages/plugin-commands-patching/test/patch.test.ts b/packages/plugin-commands-patching/test/patch.test.ts index 4aaf33b9e86..ae220aa3385 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,22 @@ 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(tempySpy.mock.results[0].value)).toBe(true) + expect(patchDir).toBe(editDir) + expect(fs.existsSync(patchDir)).toBe(true) + }) + + 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') // If editDir already exists, it should throw an error - await expect(patchFn()).rejects.toThrow(`The target directory already exists: '${editDir}'`) + await expect(() => patch.handler({ ...defaultPatchOption, editDir }, ['is-positive@1.0.0'])) + .rejects.toThrow(`The target directory already exists: '${editDir}'`) }) }) From e20a351980f455c3cf356c457effe92e0ee71f2f Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 10 Sep 2022 16:52:00 +0300 Subject: [PATCH 3/3] refactor: test --- packages/plugin-commands-patching/test/patch.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/plugin-commands-patching/test/patch.test.ts b/packages/plugin-commands-patching/test/patch.test.ts index ae220aa3385..c6b6169166a 100644 --- a/packages/plugin-commands-patching/test/patch.test.ts +++ b/packages/plugin-commands-patching/test/patch.test.ts @@ -75,13 +75,21 @@ describe('patch and commit', () => { expect(patchDir).toBe(editDir) expect(fs.existsSync(patchDir)).toBe(true) + + 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') - // If editDir already exists, it should throw an error await expect(() => patch.handler({ ...defaultPatchOption, editDir }, ['is-positive@1.0.0'])) .rejects.toThrow(`The target directory already exists: '${editDir}'`) })