fix(plugin-commands-patching): patch should works correctly when shared-workspace-file is false (#6889)

.changeset/cyan-oranges-wait.md

@@ -0,0 +1,9 @@
+---
+"@pnpm/plugin-commands-installation": patch
+"@pnpm/filter-workspace-packages": patch
+"@pnpm/plugin-commands-patching": patch
+"@pnpm/workspace.find-packages": patch
+"pnpm": patch
+---
+
+`pnpm patch` should works correctly when shared-workspace-file is false [#6885](https://github.com/pnpm/pnpm/issues/6885)

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

@@ -64,7 +64,7 @@ export async function handler (opts: install.InstallCommandOptions & Pick<Config
   await fs.promises.writeFile(path.join(patchesDir, `${patchFileName}.patch`), patchContent, 'utf8')
   const { writeProjectManifest, manifest } = await tryReadProjectManifest(lockfileDir)
 
-  const rootProjectManifest = opts.rootProjectManifest ?? manifest ?? {}
+  const rootProjectManifest = (!opts.sharedWorkspaceLockfile ? manifest : (opts.rootProjectManifest ?? manifest)) ?? {}
 
   if (!rootProjectManifest.pnpm) {
     rootProjectManifest.pnpm = {

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

@@ -482,7 +482,7 @@ describe('patch and commit in workspaces', () => {
   let cacheDir: string
   let storeDir: string
 
-  beforeEach(() => {
+  beforeEach(async () => {
     preparePackages([
       {
         location: '.',
@@ -516,10 +516,10 @@ describe('patch and commit in workspaces', () => {
       dir: process.cwd(),
       storeDir,
     }
+    await writeYamlFile('pnpm-workspace.yaml', { packages: ['project-1', 'project-2'] })
   })
 
   test('patch commit should work in workspaces', async () => {
-    await writeYamlFile('pnpm-workspace.yaml', { packages: ['project-1', 'project-2'] })
     const { allProjects, allProjectsGraph, selectedProjectsGraph } = await readProjects(process.cwd(), [])
     await install.handler({
       ...DEFAULT_OPTS,
@@ -572,6 +572,65 @@ describe('patch and commit in workspaces', () => {
     expect(fs.readFileSync('project-2/node_modules/is-positive/index.js', 'utf8')).toContain('// test patching')
     expect(fs.existsSync('project-2/node_modules/is-positive/license')).toBe(false)
   })
+
+  test('patch and patch-commit should work with shared-workspace-lockfile=false', async () => {
+    const { allProjects, allProjectsGraph, selectedProjectsGraph } = await readProjects(process.cwd(), [])
+    await install.handler({
+      ...DEFAULT_OPTS,
+      cacheDir,
+      storeDir,
+      allProjects,
+      allProjectsGraph,
+      dir: process.cwd(),
+      lockfileDir: undefined,
+      selectedProjectsGraph,
+      workspaceDir: process.cwd(),
+      saveLockfile: true,
+      sharedWorkspaceLockfile: false,
+    })
+    process.chdir('./project-1')
+    const output = await patch.handler({
+      ...defaultPatchOption,
+      dir: process.cwd(),
+    }, ['is-positive@1.0.0'])
+    const patchDir = getPatchDirFromPatchOutput(output)
+    const tempDir = os.tmpdir()
+
+    expect(patchDir).toContain(tempDir)
+    expect(fs.existsSync(patchDir)).toBe(true)
+
+    expect(fs.readFileSync(path.join(patchDir, 'license'), 'utf8')).toContain('The MIT License (MIT)')
+
+    fs.appendFileSync(path.join(patchDir, 'index.js'), '// test patching', 'utf8')
+    fs.unlinkSync(path.join(patchDir, 'license'))
+    await patchCommit.handler({
+      ...DEFAULT_OPTS,
+      allProjects,
+      allProjectsGraph,
+      selectedProjectsGraph,
+      dir: process.cwd(),
+      cacheDir,
+      storeDir,
+      lockfileDir: process.cwd(),
+      workspaceDir: process.cwd(),
+      saveLockfile: true,
+      frozenLockfile: false,
+      fixLockfile: true,
+      sharedWorkspaceLockfile: false,
+    }, [patchDir])
+
+    const { manifest } = await readProjectManifest(process.cwd())
+    expect(manifest.pnpm?.patchedDependencies).toStrictEqual({
+      'is-positive@1.0.0': 'patches/is-positive@1.0.0.patch',
+    })
+    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')
+    expect(fs.existsSync('./node_modules/is-positive/license')).toBe(false)
+    expect(fs.readFileSync('../project-2/node_modules/is-positive/index.js', 'utf8')).not.toContain('// test patching')
+    expect(fs.existsSync('../project-2/node_modules/is-positive/license')).toBe(true)
+  })
 })
 
 describe('patch with custom modules-dir and virtual-store-dir', () => {

pkg-manager/plugin-commands-installation/src/import/index.ts

@@ -89,6 +89,7 @@ export type ImportCommandOptions = Pick<Config,
 | 'selectedProjectsGraph'
 | 'workspaceDir'
 | 'ignoreWorkspaceCycles'
+| 'sharedWorkspaceLockfile'
 > & CreateStoreControllerOptions & Omit<InstallOptions, 'storeController' | 'lockfileOnly' | 'preferredVersions'>
 
 export async function handler (

pkg-manager/plugin-commands-installation/src/link.ts

@@ -91,6 +91,7 @@ export async function handler (
   | 'saveOptional'
   | 'saveProd'
   | 'workspaceDir'
+  | 'sharedWorkspaceLockfile'
   > & Partial<Pick<Config, 'linkWorkspacePackages'>>,
   params?: string[]
 ) {

pkg-manager/plugin-commands-installation/src/remove.ts

@@ -148,6 +148,7 @@ export async function handler (
   | 'saveProd'
   | 'selectedProjectsGraph'
   | 'workspaceDir'
+  | 'sharedWorkspaceLockfile'
   > & {
     recursive?: boolean
   },

pnpm/src/main.ts

@@ -205,6 +205,7 @@ export async function main (inputArgv: string[]) {
       testPattern: config.testPattern,
       changedFilesIgnorePattern: config.changedFilesIgnorePattern,
       useGlobDirFiltering: !config.legacyDirFiltering,
+      sharedWorkspaceLockfile: config.sharedWorkspaceLockfile,
     })
 
     if (filterResults.allProjects.length === 0) {

workspace/filter-workspace-packages/src/index.ts

@@ -64,6 +64,7 @@ export interface FilterPackagesOptions {
   testPattern?: string[]
   changedFilesIgnorePattern?: string[]
   useGlobDirFiltering?: boolean
+  sharedWorkspaceLockfile?: boolean
 }
 
 export async function filterPackagesFromDir (
@@ -71,7 +72,7 @@ export async function filterPackagesFromDir (
   filter: WorkspaceFilter[],
   opts: FilterPackagesOptions & { engineStrict?: boolean, patterns: string[] }
 ) {
-  const allProjects = await findWorkspacePackages(workspaceDir, { engineStrict: opts?.engineStrict, patterns: opts.patterns })
+  const allProjects = await findWorkspacePackages(workspaceDir, { engineStrict: opts?.engineStrict, patterns: opts.patterns, sharedWorkspaceLockfile: opts.sharedWorkspaceLockfile })
   return {
     allProjects,
     ...(await filterPackages(allProjects, filter, opts)),

workspace/find-packages/src/index.ts

@@ -15,12 +15,14 @@ export async function findWorkspacePackages (
     engineStrict?: boolean
     nodeVersion?: string
     patterns?: string[]
+    sharedWorkspaceLockfile?: boolean
   }
 ): Promise<Project[]> {
   const pkgs = await findWorkspacePackagesNoCheck(workspaceRoot, opts)
   for (const pkg of pkgs) {
     packageIsInstallable(pkg.dir, pkg.manifest, opts ?? {})
-    if (pkg.dir !== workspaceRoot) {
+    // When setting shared-workspace-lockfile=false, `pnpm` can be set in sub-project's package.json.
+    if (opts?.sharedWorkspaceLockfile && pkg.dir !== workspaceRoot) {
       checkNonRootProjectManifest(pkg)
     }
   }

workspace/find-packages/test/index.ts

@@ -38,7 +38,9 @@ test('findWorkspacePackagesNoCheck() skips engine checks', async () => {
 test('findWorkspacePackages() output warnings for non-root workspace project', async () => {
   const fixturePath = path.join(__dirname, '__fixtures__/warning-for-non-root-project')
 
-  const pkgs = await findWorkspacePackages(fixturePath)
+  const pkgs = await findWorkspacePackages(fixturePath, {
+    sharedWorkspaceLockfile: true,
+  })
   expect(pkgs.length).toBe(3)
   expect(logger.warn).toBeCalledTimes(3)
   const fooPath = path.join(fixturePath, 'packages/foo')