initial support for applying patch sequence

Author: ds300

integration-tests/apply-multiple-patches/__snapshots__/apply-multiple-patches.test.ts.snap

@@ -0,0 +1,38 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Test apply-multiple-patches: patch-package fails when a patch in the sequence is invalid 1`] = `
+"SNAPSHOT: patch-package fails when a patch in the sequence is invalid
+
+**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+03+broken.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"
+`;
+
+exports[`Test apply-multiple-patches: patch-package happily applies both good patches 1`] = `
+"SNAPSHOT: patch-package happily applies both good patches
+patch-package 0.0.0
+Applying patches...
+left-pad@1.3.0 (1 hello) ✔
+left-pad@1.3.0 (2 world) ✔
+END SNAPSHOT"
+`;

integration-tests/apply-multiple-patches/apply-multiple-patches.sh

@@ -0,0 +1,19 @@
+# make sure errors stop the script
+set -e
+
+echo "add patch-package"
+npm add $1
+alias patch-package=./node_modules/.bin/patch-package
+
+echo "SNAPSHOT: patch-package happily applies both good patches"
+patch-package
+echo "END SNAPSHOT"
+
+cp *broken.patch patches/
+
+(>&2 echo "SNAPSHOT: patch-package fails when a patch in the sequence is invalid")
+if patch-package
+then
+  exit 1
+fi
+(>&2 echo "END SNAPSHOT")

integration-tests/apply-multiple-patches/apply-multiple-patches.test.ts

@@ -0,0 +1,5 @@
+import { runIntegrationTest } from "../runIntegrationTest"
+runIntegrationTest({
+  projectName: "apply-multiple-patches",
+  shouldProduceSnapshots: true,
+})

integration-tests/apply-multiple-patches/left-pad+1.3.0+03+broken.patch

@@ -0,0 +1,9 @@
+diff --git a/node_modules/left-pad/index.js b/node_modules/left-pad/index.js
+index e90aec3..409dad7 100644
+--- a/node_modules/left-pad/index.js
++++ b/node_modules/left-pad/index.js
+@@ -1,3 +1,4 @@
++// hello. this is the first patch
+ /* Thos program is free software. It comes without any warranty, to
+      * the extent permitted by applicable law. You can redistribute it
+      * and/or modify it under the terms of the Do What The Fuck You Want

integration-tests/apply-multiple-patches/package-lock.json

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

integration-tests/apply-multiple-patches/package.json

@@ -0,0 +1,9 @@
+diff --git a/node_modules/left-pad/index.js b/node_modules/left-pad/index.js
+index e90aec3..409dad7 100644
+--- a/node_modules/left-pad/index.js
++++ b/node_modules/left-pad/index.js
+@@ -1,3 +1,4 @@
++// hello. this is the first patch
+ /* This program is free software. It comes without any warranty, to
+      * the extent permitted by applicable law. You can redistribute it
+      * and/or modify it under the terms of the Do What The Fuck You Want

integration-tests/apply-multiple-patches/patches/left-pad+1.3.0+01+hello.patch

@@ -0,0 +1,9 @@
+diff --git a/node_modules/left-pad/index.js b/node_modules/left-pad/index.js
+index e90aec3..409dad7 100644
+--- a/node_modules/left-pad/index.js
++++ b/node_modules/left-pad/index.js
+@@ -1,3 +1,4 @@
++// hello. this is the first patch
+ /* This program is free software. It comes without any warranty, to
+      * the extent permitted by applicable law. You can redistribute it
+      * and/or modify it under the terms of the Do What The Fuck You Want

integration-tests/apply-multiple-patches/patches/left-pad+1.3.0+02+world.patch

@@ -97,6 +97,8 @@ Object {
   "patchFilename": "banana++apple+0.4.2.patch",
   "path": "node_modules/banana/node_modules/apple",
   "pathSpecifier": "banana/apple",
+  "sequenceName": undefined,
+  "sequenceNumber": undefined,
   "version": "0.4.2",
 }
 `)
@@ -119,6 +121,8 @@ Object {
   "patchFilename": "@types+banana++@types+apple++@mollusc+man+0.4.2-banana-tree.patch",
   "path": "node_modules/@types/banana/node_modules/@types/apple/node_modules/@mollusc/man",
   "pathSpecifier": "@types/banana/@types/apple/@mollusc/man",
+  "sequenceName": undefined,
+  "sequenceNumber": undefined,
   "version": "0.4.2-banana-tree",
 }
 `)
@@ -140,6 +144,8 @@ Object {
   "patchFilename": "@types+banana.patch++hello+0.4.2-banana-tree.patch",
   "path": "node_modules/@types/banana.patch/node_modules/hello",
   "pathSpecifier": "@types/banana.patch/hello",
+  "sequenceName": undefined,
+  "sequenceNumber": undefined,
   "version": "0.4.2-banana-tree",
 }
 `)
@@ -161,8 +167,53 @@ Object {
   "patchFilename": "@types+banana.patch++hello+0.4.2-banana-tree.dev.patch",
   "path": "node_modules/@types/banana.patch/node_modules/hello",
   "pathSpecifier": "@types/banana.patch/hello",
+  "sequenceName": undefined,
+  "sequenceNumber": undefined,
   "version": "0.4.2-banana-tree",
 }
+`)
+  })
+
+  it("works for ordered patches", () => {
+    expect(getPackageDetailsFromPatchFilename("left-pad+1.3.0+02+world"))
+      .toMatchInlineSnapshot(`
+Object {
+  "humanReadablePathSpecifier": "left-pad",
+  "isDevOnly": false,
+  "isNested": false,
+  "name": "left-pad",
+  "packageNames": Array [
+    "left-pad",
+  ],
+  "patchFilename": "left-pad+1.3.0+02+world",
+  "path": "node_modules/left-pad",
+  "pathSpecifier": "left-pad",
+  "sequenceName": "world",
+  "sequenceNumber": 2,
+  "version": "1.3.0",
+}
+`)
+
+    expect(
+      getPackageDetailsFromPatchFilename(
+        "@microsoft/api-extractor+2.0.0+01+FixThing",
+      ),
+    ).toMatchInlineSnapshot(`
+Object {
+  "humanReadablePathSpecifier": "@microsoft/api-extractor",
+  "isDevOnly": false,
+  "isNested": false,
+  "name": "@microsoft/api-extractor",
+  "packageNames": Array [
+    "@microsoft/api-extractor",
+  ],
+  "patchFilename": "@microsoft/api-extractor+2.0.0+01+FixThing",
+  "path": "node_modules/@microsoft/api-extractor",
+  "pathSpecifier": "@microsoft/api-extractor",
+  "sequenceName": "FixThing",
+  "sequenceNumber": 1,
+  "version": "2.0.0",
+}
 `)
   })
 })

src/PackageDetails.test.ts

@@ -13,6 +13,8 @@ export interface PatchedPackageDetails extends PackageDetails {
   version: string
   patchFilename: string
   isDevOnly: boolean
+  sequenceName?: string
+  sequenceNumber?: number
 }
 
 export function parseNameAndVersion(
@@ -89,27 +91,6 @@ export function parseNameAndVersion(
 export function getPackageDetailsFromPatchFilename(
   patchFilename: string,
 ): PatchedPackageDetails | null {
-  const legacyMatch = patchFilename.match(
-    /^([^+=]+?)(:|\+)(\d+\.\d+\.\d+.*?)(\.dev)?\.patch$/,
-  )
-
-  if (legacyMatch) {
-    const name = legacyMatch[1]
-    const version = legacyMatch[3]
-
-    return {
-      packageNames: [name],
-      pathSpecifier: name,
-      humanReadablePathSpecifier: name,
-      path: join("node_modules", name),
-      name,
-      version,
-      isNested: false,
-      patchFilename,
-      isDevOnly: patchFilename.endsWith(".dev.patch"),
-    }
-  }
-
   const parts = patchFilename
     .replace(/(\.dev)?\.patch$/, "")
     .split("++")
@@ -141,6 +122,8 @@ export function getPackageDetailsFromPatchFilename(
     isNested: parts.length > 1,
     packageNames: parts.map(({ packageName: name }) => name),
     isDevOnly: patchFilename.endsWith(".dev.patch"),
+    sequenceName: lastPart.sequenceName,
+    sequenceNumber: lastPart.sequenceNumber,
   }
 }
 

src/PackageDetails.ts

@@ -7,6 +7,7 @@ import { posix } from "path"
 import {
   getPackageDetailsFromPatchFilename,
   PackageDetails,
+  PatchedPackageDetails,
 } from "./PackageDetails"
 import { reversePatch } from "./patch/reverse"
 import semver from "semver"
@@ -103,99 +104,126 @@ export function applyPatchesForApp({
   const errors: string[] = []
   const warnings: string[] = []
 
-  for (const filename of files) {
-    try {
-      const packageDetails = getPackageDetailsFromPatchFilename(filename)
-
-      if (!packageDetails) {
-        warnings.push(
-          `Unrecognized patch file in patches directory ${filename}`,
-        )
-        continue
-      }
-
-      const {
-        name,
-        version,
-        path,
-        pathSpecifier,
-        isDevOnly,
-        patchFilename,
-      } = packageDetails
-
-      const installedPackageVersion = getInstalledPackageVersion({
-        appPath,
-        path,
-        pathSpecifier,
-        isDevOnly:
-          isDevOnly ||
-          // check for direct-dependents in prod
-          (process.env.NODE_ENV === "production" &&
-            packageIsDevDependency({ appPath, packageDetails })),
-        patchFilename,
-      })
-      if (!installedPackageVersion) {
-        // it's ok we're in production mode and this is a dev only package
-        console.log(
-          `Skipping dev-only ${chalk.bold(
-            pathSpecifier,
-          )}@${version} ${chalk.blue("✔")}`,
-        )
-        continue
-      }
+  const groupedPatchFileDetails: Record<string, PatchedPackageDetails[]> = {}
+  for (const file of files) {
+    const details = getPackageDetailsFromPatchFilename(file)
+    if (!details) {
+      warnings.push(`Unrecognized patch file in patches directory ${file}`)
+      continue
+    }
+    if (!groupedPatchFileDetails[details.pathSpecifier]) {
+      groupedPatchFileDetails[details.pathSpecifier] = []
+    }
+    groupedPatchFileDetails[details.pathSpecifier].push(details)
+  }
 
-      if (
-        applyPatch({
-          patchFilePath: resolve(patchesDirectory, filename) as string,
-          reverse,
-          packageDetails,
-          patchDir,
+  for (const [_, details] of Object.entries(groupedPatchFileDetails)) {
+    details.sort((a, b) => {
+      return (a.sequenceNumber ?? 0) - (b.sequenceNumber ?? 0)
+    })
+    packageLoop: for (const packageDetails of details) {
+      try {
+        const {
+          name,
+          version,
+          path,
+          pathSpecifier,
+          isDevOnly,
+          patchFilename,
+        } = packageDetails
+
+        const installedPackageVersion = getInstalledPackageVersion({
+          appPath,
+          path,
+          pathSpecifier,
+          isDevOnly:
+            isDevOnly ||
+            // check for direct-dependents in prod
+            (process.env.NODE_ENV === "production" &&
+              packageIsDevDependency({ appPath, packageDetails })),
+          patchFilename,
         })
-      ) {
-        // yay patch was applied successfully
-        // print warning if version mismatch
-        if (installedPackageVersion !== version) {
-          warnings.push(
-            createVersionMismatchWarning({
+        if (!installedPackageVersion) {
+          // it's ok we're in production mode and this is a dev only package
+          console.log(
+            `Skipping dev-only ${chalk.bold(
+              pathSpecifier,
+            )}@${version} ${chalk.blue("✔")}`,
+          )
+          continue
+        }
+
+        if (
+          applyPatch({
+            patchFilePath: resolve(patchesDirectory, patchFilename) as string,
+            reverse,
+            packageDetails,
+            patchDir,
+          })
+        ) {
+          // yay patch was applied successfully
+          // print warning if version mismatch
+          if (installedPackageVersion !== version) {
+            warnings.push(
+              createVersionMismatchWarning({
+                packageName: name,
+                actualVersion: installedPackageVersion,
+                originalVersion: version,
+                pathSpecifier,
+                path,
+              }),
+            )
+          }
+          const sequenceString =
+            packageDetails.sequenceNumber != null
+              ? ` (${packageDetails.sequenceNumber}${
+                  packageDetails.sequenceName
+                    ? " " + packageDetails.sequenceName
+                    : ""
+                })`
+              : ""
+          console.log(
+            `${chalk.bold(
+              pathSpecifier,
+            )}@${version}${sequenceString} ${chalk.green("✔")}`,
+          )
+        } else if (installedPackageVersion === version) {
+          // completely failed to apply patch
+          // TODO: propagate useful error messages from patch application
+          errors.push(
+            createBrokenPatchFileError({
+              packageName: name,
+              patchFilename,
+              pathSpecifier,
+              path,
+            }),
+          )
+        } else {
+          errors.push(
+            createPatchApplicationFailureError({
               packageName: name,
               actualVersion: installedPackageVersion,
               originalVersion: version,
-              pathSpecifier,
+              patchFilename,
               path,
+              pathSpecifier,
             }),
           )
         }
-        console.log(
-          `${chalk.bold(pathSpecifier)}@${version} ${chalk.green("✔")}`,
-        )
-      } else if (installedPackageVersion === version) {
-        // completely failed to apply patch
-        // TODO: propagate useful error messages from patch application
-        errors.push(
-          createBrokenPatchFileError({
-            packageName: name,
-            patchFileName: filename,
-            pathSpecifier,
-            path,
-          }),
-        )
-      } else {
-        errors.push(
-          createPatchApplictionFailureError({
-            packageName: name,
-            actualVersion: installedPackageVersion,
-            originalVersion: version,
-            patchFileName: filename,
-            path,
-            pathSpecifier,
-          }),
-        )
-      }
-    } catch (error) {
-      if (error instanceof PatchApplicationError) {
-        errors.push(error.message)
-      } else {
-        errors.push(createUnexpectedError({ filename, error }))
+      } catch (error) {
+        if (error instanceof PatchApplicationError) {
+          errors.push(error.message)
+        } else {
+          errors.push(
+            createUnexpectedError({
+              filename: packageDetails.patchFilename,
+              error: error as Error,
+            }),
+          )
+        }
+        if (details.length > 0) {
+          continue packageLoop
+        }
       }
     }
   }
@@ -302,12 +330,12 @@ ${chalk.yellow("Warning:")} patch-package detected a patch file version mismatch
 
 function createBrokenPatchFileError({
   packageName,
-  patchFileName,
+  patchFilename,
   path,
   pathSpecifier,
 }: {
   packageName: string
-  patchFileName: string
+  patchFilename: string
   path: string
   pathSpecifier: string
 }) {
@@ -320,7 +348,7 @@ ${chalk.red.bold("**ERROR**")} ${chalk.red(
 
   This error was caused because patch-package cannot apply the following patch file:
 
-    patches/${patchFileName}
+    patches/${patchFilename}
 
   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
@@ -336,18 +364,18 @@ ${chalk.red.bold("**ERROR**")} ${chalk.red(
 `
 }
 
-function createPatchApplictionFailureError({
+function createPatchApplicationFailureError({
   packageName,
   actualVersion,
   originalVersion,
-  patchFileName,
+  patchFilename,
   path,
   pathSpecifier,
 }: {
   packageName: string
   actualVersion: string
   originalVersion: string
-  patchFileName: string
+  patchFilename: string
   path: string
   pathSpecifier: string
 }) {
@@ -376,7 +404,7 @@ ${chalk.red.bold("**ERROR**")} ${chalk.red(
     patch-package ${pathSpecifier}
 
   Info:
-    Patch file: patches/${patchFileName}
+    Patch file: patches/${patchFilename}
     Patch was made for version: ${chalk.green.bold(originalVersion)}
     Installed version: ${chalk.red.bold(actualVersion)}
 `