patch parse error handling. Closes ds300#131

Author: ds300

package.json

@@ -46,40 +46,39 @@
   },
   "devDependencies": {
     "@types/app-root-path": "^1.2.4",
-    "@types/chalk": "^0.4.31",
     "@types/cross-spawn": "^6.0.0",
-    "@types/fs-extra": "^4.0.0",
+    "@types/fs-extra": "^5.0.5",
     "@types/is-ci": "^1.1.0",
-    "@types/jest": "^23.3.12",
+    "@types/jest": "^24.0.11",
     "@types/minimist": "^1.2.0",
-    "@types/node": "^7.0.18",
-    "@types/rimraf": "^0.0.28",
+    "@types/node": "^11.11.6",
+    "@types/rimraf": "^2.0.2",
     "@types/semver": "^5.5.0",
-    "@types/tmp": "^0.0.33",
-    "husky": "^0.13.3",
-    "jest": "^23.6.0",
-    "lint-staged": "^3.4.1",
-    "np": "^2.15.0",
-    "prettier": "^1.15.3",
+    "@types/tmp": "^0.0.34",
+    "husky": "^1.3.1",
+    "jest": "^24.5.0",
+    "lint-staged": "^8.1.5",
+    "np": "^4.0.2",
+    "prettier": "^1.16.4",
     "randomstring": "^1.1.5",
-    "ts-jest": "^23.10.5",
-    "ts-node": "6",
-    "tslint": "^5.2.0",
-    "typescript": "^3.2.2"
+    "ts-jest": "^24.0.0",
+    "ts-node": "8.0.3",
+    "tslint": "^5.14.0",
+    "typescript": "^3.4.0-rc"
   },
   "dependencies": {
     "@yarnpkg/lockfile": "^1.1.0",
-    "chalk": "^1.1.3",
+    "chalk": "^2.4.2",
     "cross-spawn": "^6.0.5",
-    "fs-extra": "^4.0.1",
+    "fs-extra": "^7.0.1",
     "is-ci": "^2.0.0",
-    "klaw-sync": "^4.0.0",
+    "klaw-sync": "^6.0.0",
     "minimist": "^1.2.0",
-    "rimraf": "^2.6.2",
+    "rimraf": "^2.6.3",
     "semver": "^5.6.0",
-    "slash": "^1.0.0",
-    "tmp": "^0.0.31",
-    "update-notifier": "^2.4.0"
+    "slash": "^2.0.0",
+    "tmp": "^0.0.33",
+    "update-notifier": "^2.5.0"
   },
   "files": [
     "index.js",

src/applyPatches.ts

@@ -1,14 +1,17 @@
-import { bold, green, red, yellow } from "chalk"
+import chalk from "chalk"
 import { getPatchFiles } from "./patchFs"
 import { executeEffects } from "./patch/apply"
-import { existsSync, readFileSync } from "fs-extra"
+import { existsSync } from "fs-extra"
 import { join, resolve } from "./path"
 import { posix } from "path"
-import { getPackageDetailsFromPatchFilename } from "./PackageDetails"
-import { parsePatchFile } from "./patch/parse"
+import {
+  getPackageDetailsFromPatchFilename,
+  PackageDetails,
+} from "./PackageDetails"
 import { reversePatch } from "./patch/reverse"
 import isCi from "is-ci"
 import semver from "semver"
+import { readPatch } from "./patch/read"
 
 // don't want to exit(1) on postinsall locally.
 // see https://github.com/ds300/patch-package/issues/86
@@ -34,9 +37,10 @@ function getInstalledPackageVersion({
   const packageDir = join(appPath, path)
   if (!existsSync(packageDir)) {
     console.log(
-      `${yellow("Warning:")} Patch file found for package ${posix.basename(
-        pathSpecifier,
-      )}` + ` which is not present at ${packageDir}`,
+      `${chalk.yellow(
+        "Warning:",
+      )} Patch file found for package ${posix.basename(pathSpecifier)}` +
+        ` which is not present at ${packageDir}`,
     )
 
     return null
@@ -47,28 +51,28 @@ function getInstalledPackageVersion({
   return semver.valid(version)
 }
 
-export const applyPatchesForApp = (
+export function applyPatchesForApp(
   appPath: string,
   reverse: boolean,
-  patchDir: string = "patches",
-): void => {
+  patchDir: string,
+): void {
   const patchesDirectory = join(appPath, patchDir)
   const files = findPatchFiles(patchesDirectory)
 
   if (files.length === 0) {
-    console.error(red("No patch files found"))
+    console.error(chalk.red("No patch files found"))
     return
   }
 
   files.forEach(filename => {
-    const details = getPackageDetailsFromPatchFilename(filename)
+    const packageDetails = getPackageDetailsFromPatchFilename(filename)
 
-    if (!details) {
+    if (!packageDetails) {
       console.warn(`Unrecognized patch file in patches directory ${filename}`)
       return
     }
 
-    const { name, version, path, pathSpecifier } = details
+    const { name, version, path, pathSpecifier } = packageDetails
 
     const installedPackageVersion = getInstalledPackageVersion({
       appPath,
@@ -80,7 +84,14 @@ export const applyPatchesForApp = (
       return
     }
 
-    if (applyPatch(resolve(patchesDirectory, filename) as string, reverse)) {
+    if (
+      applyPatch({
+        patchFilePath: resolve(patchesDirectory, filename) as string,
+        reverse,
+        packageDetails,
+        patchDir,
+      })
+    ) {
       // yay patch was applied successfully
       // print warning if version mismatch
       if (installedPackageVersion !== version) {
@@ -92,7 +103,9 @@ export const applyPatchesForApp = (
           path,
         })
       } else {
-        console.log(`${bold(pathSpecifier)}@${version} ${green("✔")}`)
+        console.log(
+          `${chalk.bold(pathSpecifier)}@${version} ${chalk.green("✔")}`,
+        )
       }
     } else {
       // completely failed to apply patch
@@ -119,12 +132,18 @@ export const applyPatchesForApp = (
   })
 }
 
-export const applyPatch = (
-  patchFilePath: string,
-  reverse: boolean,
-): boolean => {
-  const patchFileContents = readFileSync(patchFilePath).toString()
-  const patch = parsePatchFile(patchFileContents)
+export function applyPatch({
+  patchFilePath,
+  reverse,
+  packageDetails,
+  patchDir,
+}: {
+  patchFilePath: string
+  reverse: boolean
+  packageDetails: PackageDetails
+  patchDir: string
+}): boolean {
+  const patch = readPatch({ patchFilePath, packageDetails, patchDir })
   try {
     executeEffects(reverse ? reversePatch(patch) : patch, { dryRun: false })
   } catch (e) {
@@ -152,18 +171,18 @@ function printVersionMismatchWarning({
   path: string
 }) {
   console.warn(`
-${red("Warning:")} patch-package detected a patch file version mismatch
+${chalk.red("Warning:")} patch-package detected a patch file version mismatch
 
   Don't worry! This is probably fine. The patch was still applied
   successfully. Here's the deets:
 
   Patch file created for
 
-    ${packageName}@${bold(originalVersion)}
+    ${packageName}@${chalk.bold(originalVersion)}
 
   applied to
 
-    ${packageName}@${bold(actualVersion)}
+    ${packageName}@${chalk.bold(actualVersion)}
   
   At path
   
@@ -173,7 +192,7 @@ ${red("Warning:")} patch-package detected a patch file version mismatch
   breakage even though the patch was applied successfully. Make sure the package
   still behaves like you expect (you wrote tests, right?) and then run
 
-    ${bold(`patch-package ${pathSpecifier}`)}
+    ${chalk.bold(`patch-package ${pathSpecifier}`)}
 
   to update the version in the patch file name and make this warning go away.
 `)
@@ -191,8 +210,8 @@ function printBrokenPatchFileError({
   pathSpecifier: string
 }) {
   console.error(`
-${red.bold("**ERROR**")} ${red(
-    `Failed to apply patch for package ${bold(packageName)} at path`,
+${chalk.red.bold("**ERROR**")} ${chalk.red(
+    `Failed to apply patch for package ${chalk.bold(packageName)} at path`,
   )}
   
     ${path}
@@ -231,13 +250,13 @@ function printPatchApplictionFailureError({
   pathSpecifier: string
 }) {
   console.error(`
-${red.bold("**ERROR**")} ${red(
-    `Failed to apply patch for package ${bold(packageName)} at path`,
+${chalk.red.bold("**ERROR**")} ${chalk.red(
+    `Failed to apply patch for package ${chalk.bold(packageName)} at path`,
   )}
   
     ${path}
 
-  This error was caused because ${bold(packageName)} has changed since you
+  This error was caused because ${chalk.bold(packageName)} has changed since you
   made the patch file for it. This introduced conflicts with your patch,
   just like a merge conflict in Git when separate incompatible changes are
   made to the same piece of code.
@@ -256,7 +275,7 @@ ${red.bold("**ERROR**")} ${red(
 
   Info:
     Patch file: patches/${patchFileName}
-    Patch was made for version: ${green.bold(originalVersion)}
-    Installed version: ${red.bold(actualVersion)}
+    Patch was made for version: ${chalk.green.bold(originalVersion)}
+    Installed version: ${chalk.red.bold(actualVersion)}
 `)
 }

src/index.ts

@@ -1,4 +1,4 @@
-import { bold, italic } from "chalk"
+import chalk from "chalk"
 import process from "process"
 import minimist from "minimist"
 
@@ -8,6 +8,8 @@ import { makePatch } from "./makePatch"
 import { makeRegExp } from "./makeRegExp"
 import { detectPackageManager } from "./detectPackageManager"
 import { join } from "./path"
+import { normalize, sep } from "path"
+import slash = require("slash")
 
 const appPath = getAppRootPath()
 const argv = minimist(process.argv.slice(2), {
@@ -23,7 +25,7 @@ const argv = minimist(process.argv.slice(2), {
 const packageNames = argv._
 
 console.log(
-  bold("patch-package"),
+  chalk.bold("patch-package"),
   // tslint:disable-next-line:no-var-requires
   require(join(__dirname, "../package.json")).version,
 )
@@ -33,32 +35,42 @@ if (argv.version || argv.v) {
 } else if (argv.help || argv.h) {
   printHelp()
 } else {
+  const packageManager = detectPackageManager(
+    appPath,
+    argv["use-yarn"] ? "yarn" : null,
+  )
+  const patchDir = slash(normalize((argv["patch-dir"] || "patches") + sep))
+  if (patchDir.startsWith("/")) {
+    throw new Error("--patch-dir must be a relative path")
+  }
   if (packageNames.length) {
-    const include = makeRegExp(
+    const includePaths = makeRegExp(
       argv.include,
       "include",
       /.*/,
       argv["case-sensitive-path-filtering"],
     )
-    const exclude = makeRegExp(
+    const excludePaths = makeRegExp(
       argv.exclude,
       "exclude",
       /package\.json$/,
       argv["case-sensitive-path-filtering"],
     )
-    packageNames.forEach((packageName: string) => {
-      makePatch(
-        packageName,
+
+    packageNames.forEach((packagePathSpecifier: string) => {
+      makePatch({
+        packagePathSpecifier,
         appPath,
-        detectPackageManager(appPath, argv["use-yarn"] ? "yarn" : null),
-        include,
-        exclude,
-        argv["patch-dir"],
-      )
+        packageManager,
+        includePaths,
+        excludePaths,
+        patchDir,
+      })
     })
   } else {
     console.log("Applying patches...")
-    applyPatchesForApp(appPath, !!argv["reverse"], argv["patch-dir"])
+    const reverse = !!argv["reverse"]
+    applyPatchesForApp(appPath, reverse, patchDir)
   }
 }
 
@@ -69,9 +81,9 @@ Usage:
   1. Patching packages
   ====================
 
-    ${bold("patch-package")}
+    ${chalk.bold("patch-package")}
 
-  Without arguments, the ${bold(
+  Without arguments, the ${chalk.bold(
     "patch-package",
   )} command will attempt to find and apply
   patch files to your project. It looks for files named like
@@ -81,34 +93,36 @@ Usage:
   2. Creating patch files
   =======================
 
-    ${bold("patch-package")} <package-name>${italic("[ <package-name>]")}
+    ${chalk.bold("patch-package")} <package-name>${chalk.italic(
+    "[ <package-name>]",
+  )}
 
   When given package names as arguments, patch-package will create patch files
   based on any changes you've made to the versions installed by yarn/npm.
 
   Options:
 
-     ${bold("--use-yarn")}
+     ${chalk.bold("--use-yarn")}
 
          By default, patch-package checks whether you use npm or yarn based on
          which lockfile you have. If you have both, it uses npm by default.
          Set this option to override that default and always use yarn.
 
-     ${bold("--exclude <regexp>")}
+     ${chalk.bold("--exclude <regexp>")}
 
          Ignore paths matching the regexp when creating patch files.
          Paths are relative to the root dir of the package to be patched.
 
          Default: 'package\\.json$'
 
-     ${bold("--include <regexp>")}
+     ${chalk.bold("--include <regexp>")}
 
          Only consider paths matching the regexp when creating patch files.
          Paths are relative to the root dir of the package to be patched.
 
          Default '.*'
 
-     ${bold("--case-sensitive-path-filtering")}
+     ${chalk.bold("--case-sensitive-path-filtering")}
 
          Make regexps used in --include or --exclude filters case-sensitive.
 `)