fix(deploy): node-linker=hoisted produces empty node_modules (#8525)

close https://github.com/pnpm/pnpm/issues/6682

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/gold-gorillas-call.md

@@ -0,0 +1,8 @@
+---
+"@pnpm/headless": patch
+"pnpm": patch
+"@pnpm/plugin-commands-installation": patch
+"@pnpm/plugin-commands-deploy": patch
+---
+
+Fix a regression in which `pnpm deploy` with `node-linker=hoisted` produces an empty `node_modules` directory [#6682](https://github.com/pnpm/pnpm/issues/6682).

.changeset/warm-dragons-rescue.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/core": patch
+---
+
+Do not save lockfile when `saveLockfile` is `false`.

pkg-manager/core/src/install/index.ts

@@ -1405,7 +1405,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
       )
     }
   } else {
-    if (opts.useLockfile && !isInstallationOnlyForLockfileCheck) {
+    if (opts.useLockfile && opts.saveLockfile && !isInstallationOnlyForLockfileCheck) {
       await writeWantedLockfile(ctx.lockfileDir, newLockfile, lockfileOpts)
     }
 

pkg-manager/direct-dep-linker/src/linkDirectDeps.ts

@@ -83,7 +83,7 @@ async function deletePkgsPresentInRoot (
 ): Promise<boolean> {
   const pkgsLinkedToCurrentProject = await readLinkedDepsWithRealLocations(modulesDir)
   const pkgsToDelete = pkgsLinkedToCurrentProject
-    .filter(({ linkedFrom }) => pkgsLinkedToRoot.some(pathsEqual.bind(null, linkedFrom)))
+    .filter(({ linkedFrom, linkedTo }) => linkedFrom !== linkedTo && pkgsLinkedToRoot.some(pathsEqual.bind(null, linkedFrom)))
   await Promise.all(pkgsToDelete.map(({ linkedTo }) => fs.promises.unlink(linkedTo)))
   return pkgsToDelete.length === pkgsLinkedToCurrentProject.length
 }

pkg-manager/headless/src/lockfileToHoistedDepGraph.ts

@@ -40,6 +40,7 @@ export interface LockfileToHoistedDepGraphOptions {
   ignoreScripts: boolean
   currentHoistedLocations?: Record<string, string[]>
   lockfileDir: string
+  modulesDir?: string
   nodeVersion: string
   pnpmVersion: string
   registries: Registries
@@ -83,7 +84,7 @@ async function _lockfileToHoistedDepGraph (
     autoInstallPeers: opts.autoInstallPeers,
   })
   const graph: DependenciesGraph = {}
-  const modulesDir = path.join(opts.lockfileDir, 'node_modules')
+  const modulesDir = path.join(opts.lockfileDir, opts.modulesDir ?? 'node_modules')
   const fetchDepsOpts = {
     ...opts,
     lockfile,

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

@@ -271,6 +271,7 @@ export type InstallCommandOptions = Pick<Config,
 | 'lockfileDir'
 | 'lockfileOnly'
 | 'modulesDir'
+| 'nodeLinker'
 | 'pnpmfile'
 | 'preferFrozenLockfile'
 | 'production'
@@ -316,7 +317,7 @@ export type InstallCommandOptions = Pick<Config,
   workspace?: boolean
   includeOnlyPackageFiles?: boolean
   confirmModulesPurge?: boolean
-} & Partial<Pick<Config, 'modulesCacheMaxAge' | 'pnpmHomeDir' | 'preferWorkspacePackages'>>
+} & Partial<Pick<Config, 'modulesCacheMaxAge' | 'pnpmHomeDir' | 'preferWorkspacePackages' | 'useLockfile'>>
 
 export async function handler (opts: InstallCommandOptions): Promise<void> {
   const include = {

releasing/plugin-commands-deploy/src/deploy.ts

@@ -56,7 +56,7 @@ export function help (): string {
 }
 
 export async function handler (
-  opts: install.InstallCommandOptions,
+  opts: Omit<install.InstallCommandOptions, 'useLockfile'>,
   params: string[]
 ): Promise<void> {
   if (!opts.workspaceDir) {
@@ -111,6 +111,9 @@ export async function handler (
     },
     frozenLockfile: false,
     preferFrozenLockfile: false,
+    // Deploy doesn't work currently with hoisted node_modules.
+    // TODO: make it work as we need to prefer packages from the lockfile during deployment.
+    useLockfile: opts.nodeLinker !== 'hoisted',
     saveLockfile: false,
     virtualStoreDir: path.join(deployDir, 'node_modules/.pnpm'),
     modulesDir: path.relative(opts.workspaceDir, path.join(deployDir, 'node_modules')),

releasing/plugin-commands-deploy/test/deploy.test.ts

@@ -146,6 +146,83 @@ test('deploy in workspace with shared-workspace-lockfile=false', async () => {
   expect(fs.existsSync('pnpm-lock.yaml')).toBeFalsy() // no changes to the lockfile are written
 })
 
+test('deploy with node-linker=hoisted', async () => {
+  preparePackages([
+    {
+      location: '.',
+      package: {
+        name: 'root',
+      },
+    },
+    {
+      name: 'project-1',
+      version: '1.0.0',
+      files: ['index.js'],
+      dependencies: {
+        'project-2': 'workspace:*',
+        'is-positive': '1.0.0',
+      },
+      devDependencies: {
+        'project-3': 'workspace:*',
+        'is-negative': '1.0.0',
+      },
+    },
+    {
+      name: 'project-2',
+      version: '2.0.0',
+      files: ['index.js'],
+      dependencies: {
+        'project-3': 'workspace:*',
+        'is-odd': '1.0.0',
+      },
+    },
+    {
+      name: 'project-3',
+      version: '2.0.0',
+      files: ['index.js'],
+      dependencies: {
+        'project-3': 'workspace:*',
+        'is-odd': '1.0.0',
+      },
+    },
+  ])
+
+  ; ['project-1', 'project-2', 'project-3'].forEach(name => {
+    fs.writeFileSync(`${name}/test.js`, '', 'utf8')
+    fs.writeFileSync(`${name}/index.js`, '', 'utf8')
+  })
+
+  const { allProjects, selectedProjectsGraph } = await filterPackagesFromDir(process.cwd(), [{ namePattern: 'project-1' }])
+
+  await deploy.handler({
+    ...DEFAULT_OPTS,
+    allProjects,
+    dir: process.cwd(),
+    dev: false,
+    production: true,
+    recursive: true,
+    selectedProjectsGraph,
+    nodeLinker: 'hoisted',
+    sharedWorkspaceLockfile: true,
+    lockfileDir: process.cwd(),
+    workspaceDir: process.cwd(),
+  }, ['dist'])
+
+  const project = assertProject(path.resolve('dist'))
+  project.has('project-2')
+  project.has('is-positive')
+  project.has('project-3')
+  project.hasNot('is-negative')
+  expect(fs.existsSync('dist/index.js')).toBeTruthy()
+  expect(fs.existsSync('dist/test.js')).toBeFalsy()
+  expect(fs.existsSync('dist/node_modules/.modules.yaml')).toBeTruthy()
+  expect(fs.existsSync('dist/node_modules/project-2/index.js')).toBeTruthy()
+  expect(fs.existsSync('dist/node_modules/project-2/test.js')).toBeFalsy()
+  expect(fs.existsSync('dist/node_modules/project-3/index.js')).toBeTruthy()
+  expect(fs.existsSync('dist/node_modules/project-3/test.js')).toBeFalsy()
+  expect(fs.existsSync('pnpm-lock.yaml')).toBeFalsy() // no changes to the lockfile are written
+})
+
 test('deploy fails when the destination directory exists and is not empty', async () => {
   preparePackages([
     {