mirror of
https://github.com/pnpm/pnpm.git
synced 2026-05-16 12:51:45 -04:00
feat!: remove deprecated patch options (#10505)
* refactor: remove allowNonAppliedPatches * refactor: remove ignorePatchFailures * refactor: remove `strict` field in groupPatchedDependencies * test: update test failure in package patching * test: fix * docs: update changesets --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
This commit is contained in:
committed by
Zoltan Kochan
parent
f8b4895e0a
commit
7b1c189f2e
@@ -1,9 +1,7 @@
|
||||
import { PnpmError } from '@pnpm/error'
|
||||
import { applyPatch } from '@pnpm/patch-package/dist/applyPatches.js'
|
||||
import { globalWarn } from '@pnpm/logger'
|
||||
|
||||
export interface ApplyPatchToDirOpts {
|
||||
allowFailure?: boolean
|
||||
patchedDir: string
|
||||
patchFilePath: string
|
||||
}
|
||||
@@ -27,12 +25,7 @@ export function applyPatchToDir (opts: ApplyPatchToDirOpts): boolean {
|
||||
process.chdir(cwd)
|
||||
}
|
||||
if (!success) {
|
||||
const message = `Could not apply patch ${opts.patchFilePath} to ${opts.patchedDir}`
|
||||
if (opts.allowFailure) {
|
||||
globalWarn(message)
|
||||
} else {
|
||||
throw new PnpmError('PATCH_FAILED', message)
|
||||
}
|
||||
throw new PnpmError('PATCH_FAILED', `Could not apply patch ${opts.patchFilePath} to ${opts.patchedDir}`)
|
||||
}
|
||||
return success
|
||||
}
|
||||
|
||||
@@ -27,15 +27,13 @@ function prepareDirToPatch () {
|
||||
return dir
|
||||
}
|
||||
|
||||
describe('applyPatchToDir() without allowFailure', () => {
|
||||
const allowFailure = false
|
||||
describe('applyPatchToDir()', () => {
|
||||
it('should succeed when patch is applicable', () => {
|
||||
const patchFilePath = f.find('applicable.patch')
|
||||
const successfullyPatched = f.find('successfully-patched.txt')
|
||||
const patchedDir = prepareDirToPatch()
|
||||
expect(
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath,
|
||||
patchedDir,
|
||||
})
|
||||
@@ -48,7 +46,6 @@ describe('applyPatchToDir() without allowFailure', () => {
|
||||
const patchedDir = prepareDirToPatch()
|
||||
expect(() => {
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath,
|
||||
patchedDir,
|
||||
})
|
||||
@@ -59,7 +56,6 @@ describe('applyPatchToDir() without allowFailure', () => {
|
||||
const patchFilePath = f.find('invalid.patch')
|
||||
expect(() => {
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath,
|
||||
patchedDir: tempDir(),
|
||||
})
|
||||
@@ -68,59 +64,6 @@ describe('applyPatchToDir() without allowFailure', () => {
|
||||
it('should fail if the patch file is not found', () => {
|
||||
expect(() => {
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath: 'does-not-exist.patch',
|
||||
patchedDir: tempDir(),
|
||||
})
|
||||
}).toThrow('Patch file not found')
|
||||
})
|
||||
})
|
||||
|
||||
describe('applyPatchToDir() with allowFailure', () => {
|
||||
const allowFailure = true
|
||||
it('should succeed when patch is applicable', () => {
|
||||
const patchFilePath = f.find('applicable.patch')
|
||||
const successfullyPatched = f.find('successfully-patched.txt')
|
||||
const patchedDir = prepareDirToPatch()
|
||||
expect(
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath,
|
||||
patchedDir,
|
||||
})
|
||||
).toBe(true)
|
||||
const patchTarget = path.join(patchedDir, 'patch-target.txt')
|
||||
expect(fs.readFileSync(patchTarget, 'utf-8')).toBe(fs.readFileSync(successfullyPatched, 'utf-8'))
|
||||
})
|
||||
it('should warn when patch fails to apply', () => {
|
||||
const patchFilePath = f.find('non-applicable.patch')
|
||||
const patchedDir = prepareDirToPatch()
|
||||
expect(
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath,
|
||||
patchedDir,
|
||||
})
|
||||
).toBe(false)
|
||||
expect(jest.mocked(globalWarn).mock.calls).toStrictEqual([[
|
||||
`Could not apply patch ${patchFilePath} to ${patchedDir}`,
|
||||
]])
|
||||
expect(fs.readFileSync(path.join(patchedDir, 'patch-target.txt'), 'utf-8')).toBe(fs.readFileSync(f.find('patch-target.txt'), 'utf-8'))
|
||||
})
|
||||
it('should fail on invalid patch', () => {
|
||||
const patchFilePath = f.find('invalid.patch')
|
||||
expect(() => {
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath,
|
||||
patchedDir: tempDir(),
|
||||
})
|
||||
}).toThrow(`Applying patch "${patchFilePath}" failed: hunk header integrity check failed`)
|
||||
})
|
||||
it('should fail if the patch file is not found', () => {
|
||||
expect(() => {
|
||||
applyPatchToDir({
|
||||
allowFailure,
|
||||
patchFilePath: 'does-not-exist.patch',
|
||||
patchedDir: tempDir(),
|
||||
})
|
||||
|
||||
@@ -22,7 +22,7 @@ export function groupPatchedDependencies (patchedDependencies: Record<string, Pa
|
||||
const { name, version, nonSemverVersion } = dp.parse(key)
|
||||
|
||||
if (name && version) {
|
||||
getGroup(name).exact[version] = { strict: true, file, key }
|
||||
getGroup(name).exact[version] = { file, key }
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -31,18 +31,17 @@ export function groupPatchedDependencies (patchedDependencies: Record<string, Pa
|
||||
throw new PnpmError('PATCH_NON_SEMVER_RANGE', `${nonSemverVersion} is not a valid semantic version range.`)
|
||||
}
|
||||
if (nonSemverVersion.trim() === '*') {
|
||||
getGroup(name).all = { strict: true, file, key }
|
||||
getGroup(name).all = { file, key }
|
||||
} else {
|
||||
getGroup(name).range.push({
|
||||
version: nonSemverVersion,
|
||||
patch: { strict: true, file, key },
|
||||
patch: { file, key },
|
||||
})
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Set `strict` to `false` to preserve backward compatibility.
|
||||
getGroup(key).all = { strict: false, file, key }
|
||||
getGroup(key).all = { file, key }
|
||||
}
|
||||
|
||||
return result
|
||||
|
||||
@@ -15,7 +15,6 @@ test('getPatchInfo() returns an exact version patch if the name and version matc
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@1.0.0',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
range: [],
|
||||
@@ -40,7 +39,6 @@ test('getPatchInfo() returns a range version patch if the name matches and the v
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@1',
|
||||
strict: true,
|
||||
},
|
||||
}],
|
||||
all: undefined,
|
||||
@@ -63,7 +61,6 @@ test('getPatchInfo() returns name-only patch if the name matches', () => {
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
} satisfies PatchGroupRecord
|
||||
@@ -83,7 +80,6 @@ test('exact version patches override version range patches, version range patche
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@1.0.0',
|
||||
strict: true,
|
||||
},
|
||||
'1.1.0': {
|
||||
file: {
|
||||
@@ -91,7 +87,6 @@ test('exact version patches override version range patches, version range patche
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@1.1.0',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
range: [
|
||||
@@ -103,7 +98,6 @@ test('exact version patches override version range patches, version range patche
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@1',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -114,7 +108,6 @@ test('exact version patches override version range patches, version range patche
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@2',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
],
|
||||
@@ -124,7 +117,6 @@ test('exact version patches override version range patches, version range patche
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
} satisfies PatchGroupRecord
|
||||
@@ -150,7 +142,6 @@ test('getPatchInfo(_, name, version) throws an error when name@version matches m
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@>=1.0.0 <3.0.0',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -161,7 +152,6 @@ test('getPatchInfo(_, name, version) throws an error when name@version matches m
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@>=2.0.0',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
],
|
||||
@@ -185,7 +175,6 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@>=1.0.0 <3.0.0',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
range: [
|
||||
@@ -197,7 +186,6 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@>=1.0.0 <3.0.0',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -208,7 +196,6 @@ test('getPatchInfo(_, name, version) does not throw an error when name@version m
|
||||
hash: '00000000000000000000000000000000',
|
||||
},
|
||||
key: 'foo@>=2.0.0',
|
||||
strict: true,
|
||||
},
|
||||
},
|
||||
],
|
||||
|
||||
@@ -53,17 +53,16 @@ test('groups patchedDependencies according to names, match types, and versions',
|
||||
path: 'patches/mixed-style.patch',
|
||||
},
|
||||
} satisfies Record<string, PatchFile>
|
||||
const info = (strict: boolean, key: keyof typeof patchedDependencies): ExtendedPatchInfo => ({
|
||||
strict,
|
||||
const info = (key: keyof typeof patchedDependencies): ExtendedPatchInfo => ({
|
||||
key,
|
||||
file: patchedDependencies[key],
|
||||
})
|
||||
expect(_groupPatchedDependencies(patchedDependencies)).toStrictEqual({
|
||||
'exact-version-only': {
|
||||
exact: {
|
||||
'0.0.0': info(true, 'exact-version-only@0.0.0'),
|
||||
'1.2.3': info(true, 'exact-version-only@1.2.3'),
|
||||
'2.1.0': info(true, 'exact-version-only@2.1.0'),
|
||||
'0.0.0': info('exact-version-only@0.0.0'),
|
||||
'1.2.3': info('exact-version-only@1.2.3'),
|
||||
'2.1.0': info('exact-version-only@2.1.0'),
|
||||
},
|
||||
range: [],
|
||||
all: undefined,
|
||||
@@ -73,11 +72,11 @@ test('groups patchedDependencies according to names, match types, and versions',
|
||||
range: [
|
||||
{
|
||||
version: '~1.2.0',
|
||||
patch: info(true, 'version-range-only@~1.2.0'),
|
||||
patch: info('version-range-only@~1.2.0'),
|
||||
},
|
||||
{
|
||||
version: '4',
|
||||
patch: info(true, 'version-range-only@4'),
|
||||
patch: info('version-range-only@4'),
|
||||
},
|
||||
],
|
||||
all: undefined,
|
||||
@@ -85,24 +84,24 @@ test('groups patchedDependencies according to names, match types, and versions',
|
||||
'star-version-range': {
|
||||
exact: {},
|
||||
range: [],
|
||||
all: info(true, 'star-version-range@*'),
|
||||
all: info('star-version-range@*'),
|
||||
},
|
||||
'without-versions': {
|
||||
exact: {},
|
||||
range: [],
|
||||
all: info(false, 'without-versions'),
|
||||
all: info('without-versions'),
|
||||
},
|
||||
'mixed-style': {
|
||||
exact: {
|
||||
'0.1.2': info(true, 'mixed-style@0.1.2'),
|
||||
'0.1.2': info('mixed-style@0.1.2'),
|
||||
},
|
||||
range: [
|
||||
{
|
||||
version: '1.x.x',
|
||||
patch: info(true, 'mixed-style@1.x.x'),
|
||||
patch: info('mixed-style@1.x.x'),
|
||||
},
|
||||
],
|
||||
all: info(false, 'mixed-style'),
|
||||
all: info('mixed-style'),
|
||||
},
|
||||
} as PatchGroupRecord)
|
||||
})
|
||||
|
||||
@@ -8,7 +8,7 @@ test('getPatchInfo(undefined, ...) returns undefined', () => {
|
||||
expect(getPatchInfo(undefined, 'foo', '1.0.0')).toBeUndefined()
|
||||
})
|
||||
|
||||
test('getPatchInfo(_, name, version) returns strict=true if name@version exists', () => {
|
||||
test('getPatchInfo(_, name, version) if name@version exists', () => {
|
||||
expect(_getPatchInfo({
|
||||
'foo@1.0.0': {
|
||||
path: 'patches/foo@1.0.0.patch',
|
||||
@@ -20,11 +20,10 @@ test('getPatchInfo(_, name, version) returns strict=true if name@version exists'
|
||||
hash: expect.any(String),
|
||||
},
|
||||
key: 'foo@1.0.0',
|
||||
strict: true,
|
||||
})
|
||||
})
|
||||
|
||||
test('getPatchInfo(_, name, version) returns strict=false if name exists and name@version does not exist', () => {
|
||||
test('getPatchInfo(_, name, version) if name exists but name@version does not exist', () => {
|
||||
expect(_getPatchInfo({
|
||||
foo: {
|
||||
path: 'patches/foo.patch',
|
||||
@@ -36,7 +35,6 @@ test('getPatchInfo(_, name, version) returns strict=false if name exists and nam
|
||||
hash: expect.any(String),
|
||||
},
|
||||
key: 'foo',
|
||||
strict: false,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -56,7 +54,6 @@ test('getPatchInfo(_, name, version) prioritizes name@version over name if both
|
||||
hash: expect.any(String),
|
||||
},
|
||||
key: 'foo@1.0.0',
|
||||
strict: true,
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -108,7 +108,6 @@ export async function handler (opts: PatchCommandOptions, params: string[]): Pro
|
||||
|
||||
if (!opts.ignoreExisting && opts.patchedDependencies) {
|
||||
tryPatchWithExistingPatchFile({
|
||||
allowFailure: patchedDep.applyToAll,
|
||||
patchedDep,
|
||||
patchedDir: editDir,
|
||||
patchedDependencies: opts.patchedDependencies,
|
||||
@@ -129,13 +128,11 @@ To commit your changes, run:
|
||||
|
||||
function tryPatchWithExistingPatchFile (
|
||||
{
|
||||
allowFailure,
|
||||
patchedDep: { applyToAll, alias, bareSpecifier },
|
||||
patchedDir,
|
||||
patchedDependencies,
|
||||
lockfileDir,
|
||||
}: {
|
||||
allowFailure: boolean
|
||||
patchedDep: GetPatchedDependencyResult
|
||||
patchedDir: string
|
||||
patchedDependencies: Record<string, string>
|
||||
@@ -157,5 +154,5 @@ function tryPatchWithExistingPatchFile (
|
||||
if (!fs.existsSync(existingPatchFilePath)) {
|
||||
throw new PnpmError('PATCH_FILE_NOT_FOUND', `Unable to find patch file ${existingPatchFilePath}`)
|
||||
}
|
||||
applyPatchToDir({ patchedDir, patchFilePath: existingPatchFilePath, allowFailure })
|
||||
applyPatchToDir({ patchedDir, patchFilePath: existingPatchFilePath })
|
||||
}
|
||||
|
||||
@@ -849,9 +849,9 @@ describe('prompt to choose version', () => {
|
||||
expect(path.basename(patchDir)).toMatch(/^chalk@\d+\.\d+\.\d+$/)
|
||||
expect(fs.existsSync(patchDir)).toBe(true)
|
||||
expect(JSON.parse(fs.readFileSync(path.join(patchDir, 'package.json'), 'utf8')).version).toBe('5.3.0')
|
||||
expect(fs.existsSync(path.join(patchDir, 'source/index.js'))).toBe(true)
|
||||
expect(fs.existsSync(path.join(patchDir, 'license'))).toBe(true)
|
||||
|
||||
fs.appendFileSync(path.join(patchDir, 'source/index.js'), '// test patching', 'utf8')
|
||||
fs.appendFileSync(path.join(patchDir, 'license'), '\ntest patching', 'utf8')
|
||||
await patchCommit.handler({
|
||||
...DEFAULT_OPTS,
|
||||
cacheDir,
|
||||
@@ -868,8 +868,8 @@ describe('prompt to choose version', () => {
|
||||
})
|
||||
const patchContent = fs.readFileSync('patches/chalk.patch', 'utf8')
|
||||
expect(patchContent).toContain('diff --git')
|
||||
expect(patchContent).toContain('// test patching')
|
||||
expect(fs.readFileSync('node_modules/.pnpm/@pnpm.e2e+requires-chalk-530@1.0.0/node_modules/chalk/source/index.js', 'utf8')).toContain('// test patching')
|
||||
expect(patchContent).toContain('test patching')
|
||||
expect(fs.readFileSync('node_modules/.pnpm/@pnpm.e2e+requires-chalk-530@1.0.0/node_modules/chalk/license', 'utf8')).toContain('test patching')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -5,7 +5,6 @@ export interface PatchFile {
|
||||
|
||||
// TODO: replace all occurrences of PatchInfo with PatchFile before the next major version is released
|
||||
export interface PatchInfo {
|
||||
strict: boolean
|
||||
file: PatchFile
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user