fix: support npm install-strategy=linked (.store layout)

src/applyPatches.ts

@@ -8,10 +8,12 @@ import { logPatchSequenceError } from "./makePatch"
 import { PackageDetails, PatchedPackageDetails } from "./PackageDetails"
 import { packageIsDevDependency } from "./packageIsDevDependency"
 import { executeEffects } from "./patch/apply"
+import { PatchFilePart } from "./patch/parse"
 import { readPatch } from "./patch/read"
 import { reversePatch } from "./patch/reverse"
 import { getGroupedPatches } from "./patchFs"
 import { join, relative } from "./path"
+import { resolvePackagePath } from "./resolvePackagePath"
 import {
   clearPatchApplicationState,
   getPatchApplicationState,
@@ -28,17 +30,19 @@ class PatchApplicationError extends Error {
 function getInstalledPackageVersion({
   appPath,
   path,
+  resolvedPath,
   pathSpecifier,
   isDevOnly,
   patchFilename,
 }: {
   appPath: string
   path: string
+  resolvedPath: string
   pathSpecifier: string
   isDevOnly: boolean
   patchFilename: string
 }): null | string {
-  const packageDir = join(appPath, path)
+  const packageDir = join(appPath, resolvedPath)
   if (!existsSync(packageDir)) {
     if (process.env.NODE_ENV === "production" && isDevOnly) {
       return null
@@ -47,7 +51,7 @@ function getInstalledPackageVersion({
     let err =
       `${chalk.red("Error:")} Patch file found for package ${posix.basename(
         pathSpecifier,
-      )}` + ` which is not present at ${relative(".", packageDir)}`
+      )}` + ` which is not present at ${relative(".", join(appPath, path))}`
 
     if (!isDevOnly && process.env.NODE_ENV === "production") {
       err += `
@@ -180,7 +184,16 @@ export function applyPatchesForPackage({
   bestEffort: boolean
 }) {
   const pathSpecifier = patches[0].pathSpecifier
-  const state = patches.length > 1 ? getPatchApplicationState(patches[0]) : null
+  const resolvedPackagePath = resolvePackagePath({
+    appPath,
+    packageDetails: patches[0],
+    version: patches[0].version,
+  })
+
+  const state =
+    patches.length > 1
+      ? getPatchApplicationState(patches[0], resolvedPackagePath)
+      : null
   const unappliedPatches = patches.slice(0)
   const appliedPatches: PatchedPackageDetails[] = []
   // if there are multiple patches to apply, we can't rely on the reverse-patch-dry-run behavior to make this operation
@@ -235,6 +248,7 @@ export function applyPatchesForPackage({
       const installedPackageVersion = getInstalledPackageVersion({
         appPath,
         path,
+        resolvedPath: resolvedPackagePath,
         pathSpecifier,
         isDevOnly:
           isDevOnly ||
@@ -264,6 +278,7 @@ export function applyPatchesForPackage({
           patchDir,
           cwd: process.cwd(),
           bestEffort,
+          resolvedPath: resolvedPackagePath,
         })
       ) {
         appliedPatches.push(patchDetails)
@@ -282,7 +297,10 @@ export function applyPatchesForPackage({
         }
         logPatchApplication(patchDetails)
       } else if (patches.length > 1) {
-        logPatchSequenceError({ patchDetails })
+        logPatchSequenceError({
+          patchDetails,
+          resolvedPath: resolvedPackagePath,
+        })
         // in case the package has multiple patches, we need to break out of this inner loop
         // because we don't want to apply more patches on top of the broken state
         failedPatch = patchDetails
@@ -338,7 +356,7 @@ export function applyPatchesForPackage({
       }
       // if we removed all the patches that were previously applied we can delete the state file
       if (appliedPatches.length === patches.length) {
-        clearPatchApplicationState(patches[0])
+        clearPatchApplicationState(patches[0], resolvedPackagePath)
       } else {
         // We failed while reversing patches and some are still in the applied state.
         // We need to update the state file to reflect that.
@@ -364,6 +382,7 @@ export function applyPatchesForPackage({
             patchFilename: patch.patchFilename,
           })),
           isRebasing: false,
+          resolvedPath: resolvedPackagePath,
         })
       }
     } else {
@@ -390,6 +409,7 @@ export function applyPatchesForPackage({
         packageDetails: patches[0],
         patches: nextState,
         isRebasing: !!failedPatch,
+        resolvedPath: resolvedPackagePath,
       })
     }
     if (failedPatch) {
@@ -405,20 +425,27 @@ export function applyPatch({
   patchDir,
   cwd,
   bestEffort,
+  resolvedPath,
 }: {
   patchFilePath: string
   reverse: boolean
   patchDetails: PackageDetails
   patchDir: string
   cwd: string
   bestEffort: boolean
+  resolvedPath?: string
 }): boolean {
   const patch = readPatch({
     patchFilePath,
     patchDetails,
     patchDir,
   })
 
+  // Remap paths if package resolved to a different location (e.g. .store)
+  if (resolvedPath && resolvedPath !== patchDetails.path) {
+    remapPatchPaths(patch, patchDetails.path, resolvedPath)
+  }
+
   const forward = reverse ? reversePatch(patch) : patch
   try {
     if (!bestEffort) {
@@ -450,6 +477,36 @@ export function applyPatch({
   return true
 }
 
+/**
+ * Remaps paths in parsed patch effects when the package resolved to a
+ * different location than expected (e.g. npm install-strategy=linked .store).
+ */
+function remapPatchPaths(
+  patch: PatchFilePart[],
+  originalPrefix: string,
+  newPrefix: string,
+): void {
+  const remap = (path: string) =>
+    path.startsWith(`${originalPrefix}/`)
+      ? newPrefix + path.slice(originalPrefix.length)
+      : path
+
+  for (const part of patch) {
+    switch (part.type) {
+      case "patch":
+      case "file deletion":
+      case "file creation":
+      case "mode change":
+        part.path = remap(part.path)
+        break
+      case "rename":
+        part.fromPath = remap(part.fromPath)
+        part.toPath = remap(part.toPath)
+        break
+    }
+  }
+}
+
 function createVersionMismatchWarning({
   packageName,
   actualVersion,

src/createIssue.ts

@@ -36,9 +36,13 @@ function parseRepoString(repository: string): VCS {
   return { org, repo, provider: "GitHub" }
 }
 
-export function getPackageVCSDetails(packageDetails: PackageDetails): VCS {
-  const repository = require(resolve(join(packageDetails.path, "package.json")))
-    .repository as undefined | string | { url: string }
+export function getPackageVCSDetails(
+  packageDetails: PackageDetails,
+  resolvedPath?: string,
+): VCS {
+  const repository = require(resolve(
+    join(resolvedPath || packageDetails.path, "package.json"),
+  )).repository as undefined | string | { url: string }
 
   if (!repository) {
     return null
@@ -121,13 +125,15 @@ export function openIssueCreationLink({
   patchFileContents,
   packageVersion,
   patchPath,
+  resolvedPath,
 }: {
   packageDetails: PackageDetails
   patchFileContents: string
   packageVersion: string
   patchPath: string
+  resolvedPath?: string
 }) {
-  const vcs = getPackageVCSDetails(packageDetails)
+  const vcs = getPackageVCSDetails(packageDetails, resolvedPath)
 
   if (!vcs) {
     console.log(

src/makePatch.ts

@@ -32,6 +32,7 @@ import {
 import { parsePatchFile } from "./patch/parse"
 import { getGroupedPatches } from "./patchFs"
 import { dirname, join, resolve } from "./path"
+import { resolvePackagePath } from "./resolvePackagePath"
 import { resolveRelativeFileDependencies } from "./resolveRelativeFileDependencies"
 import { spawnSafeSync } from "./spawnSafe"
 import {
@@ -80,7 +81,19 @@ export function makePatch({
     return
   }
 
-  const state = getPatchApplicationState(packageDetails)
+  const resolvedPackagePath = resolvePackagePath({ appPath, packageDetails })
+  const packagePath = join(appPath, resolvedPackagePath)
+  const packageJsonPath = join(packagePath, "package.json")
+
+  if (!existsSync(packageJsonPath)) {
+    printNoPackageFoundError(
+      packagePathSpecifier,
+      join(appPath, packageDetails.path, "package.json"),
+    )
+    process.exit(1)
+  }
+
+  const state = getPatchApplicationState(packageDetails, resolvedPackagePath)
   const isRebasing = state?.isRebasing ?? false
 
   // If we are rebasing and no patches have been applied, --append is the only valid option because
@@ -137,21 +150,14 @@ export function makePatch({
     mode.type === "append" || existingPatches.length === 0
       ? existingPatches.length + 1
       : existingPatches.length
-  const vcs = getPackageVCSDetails(packageDetails)
+  const vcs = getPackageVCSDetails(packageDetails, resolvedPackagePath)
   const canCreateIssue =
     !isRebasing &&
     shouldRecommendIssue(vcs) &&
     numPatchesAfterCreate === 1 &&
     mode.type !== "append"
 
   const appPackageJson = require(join(appPath, "package.json"))
-  const packagePath = join(appPath, packageDetails.path)
-  const packageJsonPath = join(packagePath, "package.json")
-
-  if (!existsSync(packageJsonPath)) {
-    printNoPackageFoundError(packagePathSpecifier, packageJsonPath)
-    process.exit(1)
-  }
 
   const tmpRepo = dirSync({ unsafeCleanup: true })
   const tmpRepoPackagePath = join(tmpRepo.name, packageDetails.path)
@@ -187,7 +193,7 @@ export function makePatch({
     )
 
     const packageVersion = getPackageVersion(
-      join(resolve(packageDetails.path), "package.json"),
+      join(resolve(resolvedPackagePath), "package.json"),
     )
 
     // copy .npmrc/.yarnrc in case packages are hosted in private registry
@@ -228,11 +234,14 @@ export function makePatch({
         chalk.grey("•"),
         `Installing ${packageDetails.name}@${packageVersion} with npm`,
       )
+      // a copied .npmrc with install-strategy=linked would make the package a symlink git can't diff
+      const npmEnv = { ...process.env, npm_config_install_strategy: "hoisted" }
       try {
         // try first without ignoring scripts in case they are required
         // this works in 99.99% of cases
         spawnSafeSync(`npm`, ["i", "--force"], {
           cwd: tmpRepoNpmRoot,
+          env: npmEnv,
           logStdErrOnError: false,
           stdio: "ignore",
         })
@@ -241,6 +250,7 @@ export function makePatch({
         // an implicit context which we haven't reproduced
         spawnSafeSync(`npm`, ["i", "--ignore-scripts", "--force"], {
           cwd: tmpRepoNpmRoot,
+          env: npmEnv,
           stdio: "ignore",
         })
       }
@@ -500,10 +510,14 @@ export function makePatch({
               reverse: false,
               cwd: process.cwd(),
               bestEffort: false,
+              resolvedPath: resolvedPackagePath,
             })
           ) {
             didFailWhileFinishingRebase = true
-            logPatchSequenceError({ patchDetails: patch })
+            logPatchSequenceError({
+              patchDetails: patch,
+              resolvedPath: resolvedPackagePath,
+            })
             nextState.push({
               patchFilename: patch.patchFilename,
               didApply: false,
@@ -527,9 +541,10 @@ export function makePatch({
         packageDetails,
         patches: nextState,
         isRebasing: didFailWhileFinishingRebase,
+        resolvedPath: resolvedPackagePath,
       })
     } else {
-      clearPatchApplicationState(packageDetails)
+      clearPatchApplicationState(packageDetails, resolvedPackagePath)
     }
 
     if (canCreateIssue) {
@@ -539,6 +554,7 @@ export function makePatch({
           patchFileContents: diffResult.stdout.toString(),
           packageVersion,
           patchPath,
+          resolvedPath: resolvedPackagePath,
         })
       } else {
         maybePrintIssueCreationPrompt(vcs, packageDetails, packageManager)
@@ -579,8 +595,10 @@ function createPatchFileName({
 
 export function logPatchSequenceError({
   patchDetails,
+  resolvedPath = patchDetails.path,
 }: {
   patchDetails: PatchedPackageDetails
+  resolvedPath?: string
 }) {
   console.log(`
 ${chalk.red.bold("⛔ ERROR")}
@@ -595,9 +613,7 @@ To partially apply the patch (if possible) and output a log of errors to fix, ru
 
   ${chalk.bold(`patch-package --partial`)}
 
-After which you should make any required changes inside ${
-    patchDetails.path
-  }, and finally run
+After which you should make any required changes inside ${resolvedPath}, and finally run
 
   ${chalk.bold(`patch-package ${patchDetails.pathSpecifier}`)}