add error-on-fail cli option

Author: ds300

README.md

@@ -61,24 +61,27 @@ files.
 
 ### Heroku
 
-For `patch-package` to work on Heroku applications, you must specify [`NPM_CONFIG_PRODUCTION=false` or `YARN_PRODUCTION=false`](https://devcenter.heroku.com/articles/nodejs-support#package-installation). See [this issue](https://github.com/ds300/patch-package/issues/130) for more details.
+For `patch-package` to work on Heroku applications, you must specify
+[`NPM_CONFIG_PRODUCTION=false` or `YARN_PRODUCTION=false`](https://devcenter.heroku.com/articles/nodejs-support#package-installation).
+See [this issue](https://github.com/ds300/patch-package/issues/130) for more
+details.
 
 ### Docker and CI
 
 - If having errors about working directory ("cannot run in wd [...]") when
   building in Docker, you might need to adjust configuration in `.npmrc`. See
   [#185](https://github.com/ds300/patch-package/issues/185).
-- In your `Dockerfile`, remember to copy over the patch files *before* running
+- In your `Dockerfile`, remember to copy over the patch files _before_ running
   `[npm|yarn] install`
 - If you cache `node_modules` rather than running `yarn install` every time,
   make sure that the `patches` dir is included in your cache key somehow.
   Otherwise if you update a patch then the change may not be reflected on
   subsequent CI runs.
-  
-  E.g., for CircleCI: before loading/saving your cache run `cat patches/* | md5 > patches.hash`
-  and then update your hash key to include a checksum of that file, 
+
+  E.g., for CircleCI: before loading/saving your cache run
+  `cat patches/* | md5 > patches.hash` and then update your hash key to include
+  a checksum of that file,
   `{{ checksum "yarn.lock" }}-{{ checksum "patches.hash" }}`.
-  
 
 ## Usage
 
@@ -154,6 +157,19 @@ Run `patch-package` without arguments to apply all patches in your project.
 
 #### Options
 
+- `--error-on-fail`
+
+  Forces patch-package to exit with code 1 after failing.
+
+  When running locally patch-package always exits with 0 by default. This
+  happens even after failing to apply patches because otherwise yarn.lock and
+  package.json might get out of sync with node_modules, which can be very
+  confusing.
+
+  `--error-on-fail` is **switched on** by default on CI.
+
+  See https://github.com/ds300/patch-package/issues/86 for background.
+
 - `--reverse`
 
   Un-applies all patches.

integration-tests/error-on-fail/__snapshots__/error-on-fail.test.ts.snap

@@ -0,0 +1,29 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Test error-on-fail: at dev time patch-package fails but returns 0 1`] = `
+"SNAPSHOT: at dev time patch-package fails but returns 0
+
+**ERROR** Failed to apply patch for package left-pad at path
+  
+    node_modules/left-pad
+
+  This error was caused because patch-package cannot apply the following patch file:
+
+    patches/left-pad+1.3.0.patch
+
+  Try removing node_modules and trying again. If that doesn't work, maybe there was
+  an accidental change made to the patch file? Try recreating it by manually
+  editing the appropriate files and running:
+  
+    patch-package left-pad
+  
+  If that doesn't work, then it's a bug in patch-package, so please submit a bug
+  report. Thanks!
+
+    https://github.com/ds300/patch-package/issues
+    
+
+---
+patch-package finished with 1 error(s).
+END SNAPSHOT"
+`;

integration-tests/error-on-fail/error-on-fail.sh

@@ -0,0 +1,22 @@
+# make sure errors stop the script
+set -e
+
+echo "add patch-package"
+yarn add $1
+alias patch-package=./node_modules/.bin/patch-package
+
+export NODE_ENV="development"
+export CI=""
+
+(>&2 echo "SNAPSHOT: at dev time patch-package fails but returns 0")
+if ! patch-package;
+then
+  exit 1
+fi
+(>&2 echo "END SNAPSHOT")
+
+echo "adding --error-on-fail forces patch-package to return 1 at dev time"
+if patch-package --error-on-fail;
+then
+  exit 1
+fi

integration-tests/error-on-fail/error-on-fail.test.ts

@@ -0,0 +1,5 @@
+import { runIntegrationTest } from "../runIntegrationTest"
+runIntegrationTest({
+  projectName: "error-on-fail",
+  shouldProduceSnapshots: true,
+})

integration-tests/error-on-fail/package.json

@@ -0,0 +1,11 @@
+{
+  "name": "error-on-fail",
+  "version": "1.0.0",
+  "description": "integration test for patch-package",
+  "main": "index.js",
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "left-pad": "^1.3.0"
+  }
+}

integration-tests/error-on-fail/patches/left-pad+1.3.0.patch

@@ -0,0 +1,13 @@
+diff --git a/node_modules/left-pad/index.js b/node_modules/left-pad/index.js
+index e90aec3..4a20773 100644
+--- a/node_modules/left-pad/index.js
++++ b/node_modules/left-pad/index.js
+@@ -4,7 +4,7 @@
+      * To Public License, Version 2, as published by Sam Hocevar. See
+      * http://www.wtfpl.net/ for more details. */
+ 'use strict';
+-module.notreal = leftPad;
++module.patchPackage = leftPad;
+ 
+ var cache = [
+   '',

integration-tests/error-on-fail/yarn.lock

@@ -0,0 +1,8 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+left-pad@^1.3.0:
+  version "1.3.0"
+  resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz#5b8a3a7765dfe001261dde915589e782f8c94d1e"
+  integrity sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==

src/applyPatches.ts

@@ -9,7 +9,6 @@ import {
   PackageDetails,
 } from "./PackageDetails"
 import { reversePatch } from "./patch/reverse"
-import isCi from "is-ci"
 import semver from "semver"
 import { readPatch } from "./patch/read"
 import { packageIsDevDependency } from "./packageIsDevDependency"
@@ -20,12 +19,6 @@ class PatchApplicationError extends Error {
   }
 }
 
-// don't want to exit(1) on postinsall locally.
-// see https://github.com/ds300/patch-package/issues/86
-const shouldExitPostinstallWithError = isCi || process.env.NODE_ENV === "test"
-
-const exit = () => process.exit(shouldExitPostinstallWithError ? 1 : 0)
-
 function findPatchFiles(patchesDirectory: string): string[] {
   if (!existsSync(patchesDirectory)) {
     return []
@@ -90,10 +83,12 @@ export function applyPatchesForApp({
   appPath,
   reverse,
   patchDir,
+  shouldExitWithError,
 }: {
   appPath: string
   reverse: boolean
   patchDir: string
+  shouldExitWithError: boolean
 }): void {
   const patchesDirectory = join(appPath, patchDir)
   const files = findPatchFiles(patchesDirectory)
@@ -227,7 +222,7 @@ export function applyPatchesForApp({
   }
 
   if (errors.length) {
-    exit()
+    process.exit(shouldExitWithError ? 1 : 0)
   }
 }
 

src/index.ts

@@ -10,6 +10,7 @@ import { detectPackageManager } from "./detectPackageManager"
 import { join } from "./path"
 import { normalize, sep } from "path"
 import slash = require("slash")
+import isCi from "is-ci"
 
 const appPath = getAppRootPath()
 const argv = minimist(process.argv.slice(2), {
@@ -19,6 +20,7 @@ const argv = minimist(process.argv.slice(2), {
     "reverse",
     "help",
     "version",
+    "error-on-fail",
   ],
   string: ["patch-dir"],
 })
@@ -69,7 +71,11 @@ if (argv.version || argv.v) {
   } else {
     console.log("Applying patches...")
     const reverse = !!argv["reverse"]
-    applyPatchesForApp({ appPath, reverse, patchDir })
+    // don't want to exit(1) on postinsall locally.
+    // see https://github.com/ds300/patch-package/issues/86
+    const shouldExitWithError =
+      !!argv["error-on-fail"] || isCi || process.env.NODE_ENV === "test"
+    applyPatchesForApp({ appPath, reverse, patchDir, shouldExitWithError })
   }
 }
 
@@ -94,6 +100,19 @@ Usage:
     ${chalk.bold("--patch-dir <dirname>")}
 
       Specify the name for the directory in which the patch files are located.
+      
+    ${chalk.bold("--error-on-fail")}
+    
+      Forces patch-package to exit with code 1 after failing.
+    
+      When running locally patch-package always exits with 0 by default.
+      This happens even after failing to apply patches because otherwise 
+      yarn.lock and package.json might get out of sync with node_modules,
+      which can be very confusing.
+      
+      --error-on-fail is ${chalk.bold("switched on")} by default on CI.
+      
+      See https://github.com/ds300/patch-package/issues/86 for background.
 
     ${chalk.bold("--reverse")}