Skip to content

Commit 56be008

Browse files
committed
fix patch format: use declaredVersion, remove command
1 parent 09b82e8 commit 56be008

File tree

2 files changed

+92
-54
lines changed

2 files changed

+92
-54
lines changed

src/getPackageResolution.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export function getPackageResolution({
5151
console.log(`patch-package/getPackageResolution: resolved filePath ${filePath}`)
5252
}
5353
return {
54+
declaredVersion,
5455
version: `file:${filePath}`,
5556
}
5657
}
@@ -91,9 +92,7 @@ export function getPackageResolution({
9192
v.version === installedVersion,
9293
)
9394

94-
const resolutions = entries.map(([_, v]) => {
95-
return v.resolved
96-
})
95+
const resolutions = entries.map(([_, v]) => v.resolved)
9796

9897
if (resolutions.length === 0) {
9998
throw new Error(
@@ -105,26 +104,39 @@ export function getPackageResolution({
105104
console.warn(
106105
`Ambigious lockfile entries for ${packageDetails.pathSpecifier}. Using version ${installedVersion}`,
107106
)
108-
return { version: installedVersion }
107+
return {
108+
declaredVersion,
109+
version: installedVersion,
110+
}
109111
}
110112

111113
if (resolutions[0]) {
112-
return { version: resolutions[0] }
114+
return {
115+
declaredVersion,
116+
version: resolutions[0]
117+
}
113118
}
114119

115120
const resolution = entries[0][0].slice(packageDetails.name.length + 1)
116121

117122
// resolve relative file path
118123
if (resolution.startsWith("file:.")) {
119124
return {
125+
declaredVersion,
120126
version: `file:${resolve(appPath, resolution.slice("file:".length))}`,
121127
}
122128
}
123-
return { version: resolution }
129+
return {
130+
declaredVersion,
131+
version: resolution,
132+
}
124133
} else if (packageManager === "pnpm") {
125134
// WORKAROUND for pnpm bug? pnpm-lock.yaml says version 1.2.3 for linked packages, not link:../../path/to/package
126135
// TODO validate: declaredVersion must not be wildcard
127-
return { version: declaredVersion }
136+
return {
137+
declaredVersion,
138+
version: declaredVersion,
139+
}
128140

129141
// TODO dont use lockfiles at all?
130142
// package versions should be pinned in /package.json, so it works with all package managers at all times
@@ -233,7 +245,10 @@ export function getPackageResolution({
233245
entry.dependencies && packageDetails.name in entry.dependencies,
234246
)
235247
const pkg = relevantStackEntry.dependencies[packageDetails.name]
236-
return { version: pkg.resolved || pkg.from || pkg.version }
248+
return {
249+
declaredVersion,
250+
version: pkg.resolved || pkg.from || pkg.version,
251+
}
237252
}
238253
}
239254

src/makePatch.ts

Lines changed: 69 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import chalk from "chalk"
22
import { join, dirname, resolve } from "./path"
3-
import { basename } from "path"
43
import { spawnSafeSync } from "./spawnSafe"
54
import { PackageManager } from "./detectPackageManager"
65
import { removeIgnoredFiles } from "./filterFiles"
@@ -12,10 +11,6 @@ import {
1211
mkdirpSync,
1312
realpathSync,
1413
renameSync,
15-
/*
16-
lstatSync,
17-
readlinkSync,
18-
*/
1914
} from "fs-extra"
2015
import { sync as rimraf } from "rimraf"
2116
import { copySync } from "fs-extra"
@@ -35,11 +30,12 @@ import {
3530
maybePrintIssueCreationPrompt,
3631
openIssueCreationLink,
3732
} from "./createIssue"
38-
import { quote as shlexQuote } from "shlex"
33+
import { spawnSync } from "child_process"
3934

35+
// globals are set in src/index.ts
4036
const isVerbose = global.patchPackageIsVerbose
4137
const isDebug = global.patchPackageIsDebug
42-
const isTest = process.env.NODE_ENV == 'test'
38+
const patchPackageVersion = global.patchPackageVersion
4339

4440
function printNoPackageFoundError(
4541
packageName: string,
@@ -331,12 +327,17 @@ in the format <major>.<minor>.<patch>, for example: "${packageDetails.name}": "$
331327
}
332328
}
333329

334-
const git = (...args: string[]) =>
335-
spawnSafeSync("git", args, {
330+
function git(...args: string[]) {
331+
if (isDebug) {
332+
const argsStr = JSON.stringify(["git", ...args])
333+
console.log(`patch-package/makePatch: spawn: args = ${argsStr} + workdir = ${tmpRepo.name}`)
334+
}
335+
return spawnSafeSync("git", args, {
336336
cwd: tmpRepo.name,
337337
env: { ...process.env, HOME: tmpRepo.name },
338338
maxBuffer: 1024 * 1024 * 100,
339339
})
340+
}
340341

341342
// remove nested node_modules just to be safe
342343
rimraf(join(tmpRepoPackagePath, "node_modules"))
@@ -347,6 +348,8 @@ in the format <major>.<minor>.<patch>, for example: "${packageDetails.name}": "$
347348
console.info(chalk.grey("•"), "Diffing your files with clean files")
348349
writeFileSync(join(tmpRepo.name, ".gitignore"), "!/node_modules\n\n")
349350
git("init")
351+
// TODO use env-vars for "git commit"
352+
// GIT_{COMMITTER,AUTHOR}_{NAME,EMAIL}
350353
git("config", "--local", "user.name", "patch-package")
351354
git("config", "--local", "user.email", "patch@pack.age")
352355

@@ -355,39 +358,84 @@ in the format <major>.<minor>.<patch>, for example: "${packageDetails.name}": "$
355358
removeIgnoredFiles(tmpRepoPackagePath, includePaths, excludePaths)
356359

357360
git("add", "-f", packageDetails.path)
361+
if (isVerbose) {
362+
console.log(`git status:\n` + git("status").stdout.toString())
363+
}
358364
git("commit", "--allow-empty", "-m", "init")
359365

360366
// replace package with user's version
367+
if (isVerbose) {
368+
console.log(`patch-package/makePatch: remove all files in ${tmpRepoPackagePath}`)
369+
}
361370
rimraf(tmpRepoPackagePath)
362371

363372
if (isVerbose) {
364-
console.log(
365-
`patch-package/makePatch: copy ${realpathSync(
366-
packagePath,
367-
)} to ${tmpRepoPackagePath}`,
368-
)
373+
console.log(`patch-package/makePatch: git status:\n` + git("status").stdout.toString())
369374
}
370375

371376
// pnpm installs packages as symlinks, copySync would copy only the symlink
377+
// with pnpm, realpath resolves to ./node_modules/.pnpm/${name}@${version}
372378
const srcPath = realpathSync(packagePath)
379+
if (isVerbose) {
380+
console.log(
381+
`patch-package/makePatch: copy ${srcPath} to ${tmpRepoPackagePath} + skip ${srcPath}/node_modules/`,
382+
)
383+
}
373384
copySync(srcPath, tmpRepoPackagePath, {
374385
filter: (path) => {
375-
return !path.startsWith(srcPath + "/node_modules/")
386+
const doCopy = !path.startsWith(srcPath + "/node_modules/")
387+
if (isVerbose) {
388+
if (doCopy) {
389+
console.log(`patch-package/makePatch: copySync: copy file ${path}`)
390+
}
391+
else {
392+
console.log(`patch-package/makePatch: copySync: skip file ${path}`)
393+
}
394+
}
395+
return doCopy
376396
},
377397
})
378398

399+
if (isDebug) {
400+
// list files
401+
// NOTE this works only on linux
402+
console.log(`patch-package/makePatch: files in srcPath = ${srcPath}`)
403+
console.log(spawnSync('find', ['.'], { cwd: srcPath, encoding: 'utf8' }).stdout)
404+
console.log(`patch-package/makePatch: files in tmpRepoPackagePath = ${tmpRepoPackagePath}`)
405+
console.log(spawnSync('find', ['.'], { cwd: tmpRepoPackagePath, encoding: 'utf8' }).stdout)
406+
}
407+
379408
// remove nested node_modules just to be safe
380409
rimraf(join(tmpRepoPackagePath, "node_modules"))
410+
381411
// remove .git just to be safe
412+
// NOTE this removes ./node_modules/${dependencyName}/.git not ./.git
382413
rimraf(join(tmpRepoPackagePath, ".git"))
383414

384415
// also remove ignored files like before
416+
// for example, remove package.json
417+
// TODO support patching package.json via semantic json diff
385418
// use CLI options --exclude and --include
419+
420+
if (isDebug) {
421+
console.log(`patch-package/makePatch: removing ignored files in tmpRepoPackagePath = ${tmpRepoPackagePath}:`)
422+
}
386423
removeIgnoredFiles(tmpRepoPackagePath, includePaths, excludePaths)
387424

425+
if (isDebug) {
426+
// list files
427+
// NOTE this works only on linux
428+
console.log(`patch-package/makePatch: files in tmpRepoPackagePath = ${tmpRepoPackagePath}:`)
429+
console.log(spawnSync('find', ['.', '-type', 'f'], { cwd: tmpRepoPackagePath, encoding: 'utf8' }).stdout)
430+
}
431+
388432
// stage all files
389433
git("add", "-f", packageDetails.path)
390434

435+
if (isVerbose) {
436+
console.log(`patch-package/makePatch: git status:\n` + git("status").stdout.toString())
437+
}
438+
391439
const ignorePaths = ["package-lock.json", "pnpm-lock.yaml"]
392440

393441
// get diff of changes
@@ -473,44 +521,19 @@ in the format <major>.<minor>.<patch>, for example: "${packageDetails.name}": "$
473521
}
474522
})
475523

476-
const patchPackageVersion = require("../package.json").version
477-
478524
// patchfiles are parsed in patch/parse.ts function parsePatchLines
479525
// -> header comments are ignored
480526
let diffHeader = ""
481527
diffHeader += `# generated by patch-package ${patchPackageVersion}\n`
482528
diffHeader += `#\n`
483-
const prettyArgv = process.argv.slice()
484-
if (prettyArgv[0].match(/node/)) {
485-
prettyArgv[0] = "npx"
486-
}
487-
if (prettyArgv[1].match(/patch-package/)) {
488-
prettyArgv[1] = "patch-package"
489-
}
490-
diffHeader += `# command:\n`
491-
diffHeader += `# ${prettyArgv.map((a) => shlexQuote(a)).join(" ")}\n`
492-
diffHeader += `#\n`
493-
diffHeader += `# declared package:\n`
494-
// TODO rename resolvedVersion.version to declaredVersion
495-
const declaredPackageStr = (
496-
isTest ? (() => {
497-
const v = resolvedVersion.version
498-
const b = basename(v)
499-
if (b != v) return `file:/mocked/path/to/${b}` // mock path // TODO keep the relative path? as declared in /package.json. see getPackageResolution "resolve relative file path"
500-
return v
501-
})() :
529+
if (isDebug) {
502530
resolvedVersion.version
503-
)
504-
diffHeader += `# ${packageDetails.name}: ${declaredPackageStr}\n`
505-
/* redundant. this is visible from command, sample: npx patch-package wrap-ansi/string-width -> packageNames: wrap-ansi, string-width
506-
if (packageDetails.packageNames.length > 1) {
507-
diffHeader += `#\n`
508-
diffHeader += `# package names:\n`
509-
packageDetails.packageNames.forEach((packageName) => {
510-
diffHeader += `# ${packageName}\n`
511-
})
512531
}
513-
*/
532+
diffHeader += `# declared package:\n`
533+
diffHeader += `# ${packageDetails.name}: ${resolvedVersion.declaredVersion}\n`
534+
// NOTE: we do *not* include the locked version from package-lock.json or yarn.lock or pnpm-lock.yaml or ...
535+
// because patch-package should work with all package managers (should be manager-agnostic)
536+
// users can pin versions in package.json
514537
diffHeader += `#\n`
515538

516539
const patchFileName = createPatchFileName({

0 commit comments

Comments
 (0)