feat(patch): update patched dependencies on install (#4905)

.changeset/eighty-apricots-poke.md

@@ -0,0 +1,5 @@
+---
+"dependency-path": minor
+---
+
+Add patchFileHash as an optional argument to createPeersFolderSuffix().

packages/core/test/install/patch.ts

@@ -32,12 +32,14 @@ test('patch package', async () => {
 
   expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// patched')
 
+  const patchFileHash = 'jnbpamcxayl5i4ehrkoext3any'
   const lockfile = await project.readLockfile()
   expect(lockfile.patchedDependencies).toStrictEqual(patchedDependencies)
+  expect(lockfile.packages[`/is-positive/1.0.0_${patchFileHash}`]).toBeTruthy()
 
   const filesIndexFile = path.join(opts.storeDir, 'files/c7/1ccf199e0fdae37aad13946b937d67bcd35fa111b84d21b3a19439cfdc2812c5d8da8a735e94c2a1ccb77b4583808ee8405313951e7146ac83ede3671dc292-index.json')
   const filesIndex = await loadJsonFile<PackageFilesIndex>(filesIndexFile)
-  const sideEffectsKey = `${ENGINE_NAME}-{}-jnbpamcxayl5i4ehrkoext3any`
+  const sideEffectsKey = `${ENGINE_NAME}-{}-${patchFileHash}`
   const patchedFileIntegrity = filesIndex.sideEffects?.[sideEffectsKey]['index.js']?.integrity
   expect(patchedFileIntegrity).toBeTruthy()
   const originalFileIntegrity = filesIndex.files['index.js'].integrity

packages/default-reporter/src/reportError.ts

@@ -225,7 +225,7 @@ function formatGenericError (errorMessage: string, stack: object) {
     try {
       prettyStack = new StackTracey(stack).asTable()
     } catch (err: any) { // eslint-disable-line
-      prettyStack = undefined
+      prettyStack = stack.toString()
     }
     if (prettyStack) {
       return {

packages/dependency-path/src/index.ts

@@ -148,8 +148,11 @@ function depPathToFilenameUnescaped (depPath: string) {
   return depPath.replace(':', '+')
 }
 
-export function createPeersFolderSuffix (peers: Array<{name: string, version: string}>): string {
-  const folderName = peers.map(({ name, version }) => `${name.replace('/', '+')}@${version}`).sort().join('+')
+export function createPeersFolderSuffix (peers: Array<{name: string, version: string}>, patchFileHash?: string): string {
+  let folderName = peers.map(({ name, version }) => `${name.replace('/', '+')}@${version}`).sort().join('+')
+  if (patchFileHash) {
+    folderName += `_${patchFileHash}`
+  }
 
   // We don't want the folder name to get too long.
   // Otherwise, an ENAMETOOLONG error might happen.

packages/lockfile-file/src/sortLockfileKeys.ts

@@ -37,14 +37,14 @@ const ROOT_KEYS_ORDER = {
   onlyBuiltDependencies: 2,
   overrides: 3,
   packageExtensionsChecksum: 4,
+  patchedDependencies: 5,
   specifiers: 10,
   dependencies: 11,
   optionalDependencies: 12,
   devDependencies: 13,
   dependenciesMeta: 14,
-  patchedDependencies: 15,
-  importers: 16,
-  packages: 17,
+  importers: 15,
+  packages: 16,
 }
 
 function compareWithPriority (priority: Record<string, number>, left: string, right: string) {

packages/plugin-commands-patching/package.json

@@ -44,6 +44,7 @@
     "@pnpm/config": "workspace:15.3.0",
     "@pnpm/parse-wanted-dependency": "workspace:3.0.0",
     "@pnpm/pick-registry-for-package": "workspace:3.0.3",
+    "@pnpm/plugin-commands-installation": "workspace:10.2.0",
     "@pnpm/read-package-json": "workspace:6.0.4",
     "@pnpm/read-project-manifest": "workspace:3.0.4",
     "@pnpm/store-connection-manager": "workspace:4.1.11",

packages/plugin-commands-patching/src/patchCommit.ts

@@ -2,6 +2,7 @@ import fs from 'fs'
 import path from 'path'
 import { docsUrl } from '@pnpm/cli-utils'
 import { types as allTypes } from '@pnpm/config'
+import { install } from '@pnpm/plugin-commands-installation'
 import { fromDir as readPackageJsonFromDir } from '@pnpm/read-package-json'
 import { tryReadProjectManifest } from '@pnpm/read-project-manifest'
 import pick from 'ramda/src/pick'
@@ -26,7 +27,7 @@ export function help () {
   })
 }
 
-export async function handler (opts: { dir: string, lockfileDir?: string }, params: string[]) {
+export async function handler (opts: install.InstallCommandOptions, params: string[]) {
   const userDir = params[0]
   const srcDir = path.join(userDir, '../source')
   const patchContent = await diffFolders(srcDir, userDir)
@@ -35,7 +36,8 @@ export async function handler (opts: { dir: string, lockfileDir?: string }, para
   await fs.promises.mkdir(patchesDir, { recursive: true })
   const patchedPkgManifest = await readPackageJsonFromDir(srcDir)
   const pkgNameAndVersion = `${patchedPkgManifest.name}@${patchedPkgManifest.version}`
-  await fs.promises.writeFile(path.join(patchesDir, `${pkgNameAndVersion}.patch`), patchContent, 'utf8')
+  const patchFileName = pkgNameAndVersion.replace('/', '__')
+  await fs.promises.writeFile(path.join(patchesDir, `${patchFileName}.patch`), patchContent, 'utf8')
   let { manifest, writeProjectManifest } = await tryReadProjectManifest(lockfileDir)
   if (!manifest) {
     manifest = {}
@@ -47,8 +49,9 @@ export async function handler (opts: { dir: string, lockfileDir?: string }, para
   } else if (!manifest.pnpm.patchedDependencies) {
     manifest.pnpm.patchedDependencies = {}
   }
-  manifest.pnpm.patchedDependencies![pkgNameAndVersion] = `patches/${pkgNameAndVersion}.patch`
+  manifest.pnpm.patchedDependencies![pkgNameAndVersion] = `patches/${patchFileName}.patch`
   await writeProjectManifest(manifest)
+  return install.handler(opts)
 }
 
 async function diffFolders (folderA: string, folderB: string) {

packages/plugin-commands-patching/test/patch.test.ts

@@ -4,9 +4,14 @@ import prepare from '@pnpm/prepare'
 import { patch, patchCommit } from '@pnpm/plugin-commands-patching'
 import readProjectManifest from '@pnpm/read-project-manifest'
 import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
+import { DEFAULT_OPTS } from './utils/index'
 
 test('patch and commit', async () => {
-  prepare()
+  prepare({
+    dependencies: {
+      'is-positive': '1.0.0',
+    },
+  })
   const cacheDir = path.resolve('cache')
   const storeDir = path.resolve('store')
 
@@ -26,6 +31,7 @@ test('patch and commit', async () => {
   fs.appendFileSync(path.join(userPatchDir, 'index.js'), '// test patching', 'utf8')
 
   await patchCommit.handler({
+    ...DEFAULT_OPTS,
     dir: process.cwd(),
   }, [userPatchDir])
 
@@ -36,4 +42,5 @@ test('patch and commit', async () => {
   const patchContent = fs.readFileSync('patches/is-positive@1.0.0.patch', 'utf8')
   expect(patchContent).toContain('diff --git')
   expect(patchContent).toContain('// test patching')
+  expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching')
 })

packages/plugin-commands-patching/test/utils/index.ts

@@ -0,0 +1,50 @@
+import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
+
+const REGISTRY = `http://localhost:${REGISTRY_MOCK_PORT}`
+
+export const DEFAULT_OPTS = {
+  alwaysAuth: false,
+  argv: {
+    original: [],
+  },
+  bail: true,
+  bin: 'node_modules/.bin',
+  ca: undefined,
+  cacheDir: '../cache',
+  cert: undefined,
+  cliOptions: {},
+  fetchRetries: 2,
+  fetchRetryFactor: 90,
+  fetchRetryMaxtimeout: 90,
+  fetchRetryMintimeout: 10,
+  filter: [] as string[],
+  httpsProxy: undefined,
+  include: {
+    dependencies: true,
+    devDependencies: true,
+    optionalDependencies: true,
+  },
+  key: undefined,
+  linkWorkspacePackages: true,
+  localAddress: undefined,
+  lock: false,
+  lockStaleDuration: 90,
+  networkConcurrency: 16,
+  offline: false,
+  pending: false,
+  pnpmfile: './.pnpmfile.cjs',
+  pnpmHomeDir: '',
+  proxy: undefined,
+  rawConfig: { registry: REGISTRY },
+  rawLocalConfig: {},
+  registries: { default: REGISTRY },
+  registry: REGISTRY,
+  sort: true,
+  storeDir: '../store',
+  strictSsl: false,
+  userAgent: 'pnpm',
+  userConfig: {},
+  useRunningStoreServer: false,
+  useStoreServer: false,
+  workspaceConcurrency: 4,
+}

packages/plugin-commands-patching/tsconfig.json

@@ -24,6 +24,9 @@
     {
       "path": "../pick-registry-for-package"
     },
+    {
+      "path": "../plugin-commands-installation"
+    },
     {
       "path": "../read-package-json"
     },

packages/resolve-dependencies/src/index.ts

@@ -184,7 +184,6 @@ export default async function (
       }
 
       const depNode = dependenciesGraph[depPath]
-      if (depNode.isPure) continue
 
       const ref = depPathToRef(depPath, {
         alias,

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -180,6 +180,11 @@ export type PkgAddress = {
   isLinkedDependency: undefined
 })
 
+export interface PatchFile {
+  path: string
+  hash: string
+}
+
 export interface ResolvedPackage {
   id: string
   resolution: Resolution
@@ -197,10 +202,7 @@ export interface ResolvedPackage {
   optionalDependencies: Set<string>
   hasBin: boolean
   hasBundledDependencies: boolean
-  patchFile?: {
-    path: string
-    hash: string
-  }
+  patchFile?: PatchFile
   prepare: boolean
   depPath: string
   requiresBuild: boolean | SafePromiseDefer<boolean>

packages/resolve-dependencies/src/resolvePeers.ts

@@ -39,6 +39,7 @@ export type PartialResolvedPackage = Pick<ResolvedPackage,
 | 'peerDependencies'
 | 'peerDependenciesMeta'
 | 'version'
+| 'patchFile'
 >
 
 export interface GenericDependenciesGraph<T extends PartialResolvedPackage> {
@@ -180,6 +181,9 @@ function resolvePeersOfNode<T extends PartialResolvedPackage> (
     isEmpty(resolvedPackage.peerDependencies)
   ) {
     ctx.pathsByNodeId[nodeId] = resolvedPackage.depPath
+    if (resolvedPackage.patchFile) {
+      ctx.pathsByNodeId[nodeId] += `_${resolvedPackage.patchFile.hash}`
+    }
     return { resolvedPeers: {}, missingPeers: [] }
   }
   if (typeof node.children === 'function') {
@@ -248,6 +252,9 @@ function resolvePeersOfNode<T extends PartialResolvedPackage> (
   let depPath: string
   if (isEmpty(allResolvedPeers)) {
     depPath = resolvedPackage.depPath
+    if (resolvedPackage.patchFile) {
+      depPath += `_${resolvedPackage.patchFile.hash}`
+    }
   } else {
     const peersFolderSuffix = createPeersFolderSuffix(
       Object.entries(allResolvedPeers)
@@ -261,7 +268,8 @@ function resolvePeersOfNode<T extends PartialResolvedPackage> (
           }
           const { name, version } = ctx.dependenciesTree[nodeId].resolvedPackage
           return { name, version }
-        })
+        }),
+      resolvedPackage.patchFile?.hash
     )
     // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
     depPath = `${resolvedPackage.depPath}${peersFolderSuffix}`

packages/resolve-dependencies/test/resolvePeers.ts

@@ -516,3 +516,82 @@ describe('unmet peer dependency issue resolved from subdependency', () => {
     expect(peerDependencyIssuesByProjects.project.bad.dep[0].resolvedFrom).toStrictEqual([{ name: 'foo', version: '1.0.0' }])
   })
 })
+
+test('patch hash is added to the dependency path together with peer deps hash', () => {
+  const { dependenciesGraph } = resolvePeers({
+    projects: [
+      {
+        directNodeIdsByAlias: {
+          foo: '>project>foo/1.0.0>',
+          bar: '>project>bar/1.0.0>',
+          qar: '>project>qar/1.0.0>',
+        },
+        topParents: [],
+        rootDir: '',
+        id: 'project',
+      },
+    ],
+    dependenciesTree: {
+      '>project>foo/1.0.0>': {
+        children: {},
+        installable: true,
+        resolvedPackage: {
+          name: 'foo',
+          depPath: 'foo/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {
+            bar: '1.0.0',
+          },
+          patchFile: {
+            hash: 'aaa',
+            path: '/patch.patch',
+          },
+        },
+        depth: 0,
+      },
+      '>project>bar/1.0.0>': {
+        children: {},
+        installable: true,
+        resolvedPackage: {
+          name: 'bar',
+          depPath: 'bar/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {},
+        },
+        depth: 0,
+      },
+      '>project>qar/1.0.0>': {
+        children: {
+          qardep: '>project>qar/1.0.0>qardep/1.0.0>',
+        },
+        installable: true,
+        resolvedPackage: {
+          name: 'qar',
+          depPath: 'qar/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {},
+          patchFile: {
+            hash: 'bbb',
+            path: 'bbb.patch',
+          },
+        },
+        depth: 0,
+      },
+      '>project>qar/1.0.0>qardep/1.0.0>': {
+        children: {},
+        installable: true,
+        resolvedPackage: {
+          name: 'qardep',
+          depPath: 'qardep/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {},
+        },
+        depth: 0,
+      },
+    },
+    virtualStoreDir: '',
+    lockfileDir: '',
+  })
+  expect(dependenciesGraph['foo/1.0.0_bar@1.0.0_aaa']).toBeTruthy()
+  expect(dependenciesGraph['qar/1.0.0_bbb']).toBeTruthy()
+})

pnpm-lock.yaml

@@ -2473,6 +2473,7 @@ importers:
       '@pnpm/logger': ^4.0.0
       '@pnpm/parse-wanted-dependency': workspace:3.0.0
       '@pnpm/pick-registry-for-package': workspace:3.0.3
+      '@pnpm/plugin-commands-installation': workspace:10.2.0
       '@pnpm/plugin-commands-patching': workspace:0.0.0
       '@pnpm/prepare': workspace:*
       '@pnpm/read-package-json': workspace:6.0.4
@@ -2490,6 +2491,7 @@ importers:
       '@pnpm/config': link:../config
       '@pnpm/parse-wanted-dependency': link:../parse-wanted-dependency
       '@pnpm/pick-registry-for-package': link:../pick-registry-for-package
+      '@pnpm/plugin-commands-installation': link:../plugin-commands-installation
       '@pnpm/read-package-json': link:../read-package-json
       '@pnpm/read-project-manifest': link:../read-project-manifest
       '@pnpm/store-connection-manager': link:../store-connection-manager