fix(verifyDepsBeforeRun): hoisted/pnp node linker (#9430)

close #9424

.changeset/fine-pears-win.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/deps.status": minor
+"pnpm": patch
+---
+
+Fix a false negative in `verify-deps-before-run` when `node-linker` is `hoisted` and there is a workspace package without dependencies and `node_modules` directory [#9424](https://github.com/pnpm/pnpm/issues/9424).

.changeset/silly-crabs-pull.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/deps.status": minor
+"pnpm": patch
+---
+
+Explicitly drop `verify-deps-before-run` support for `node-linker=pnp`. Combining `verify-deps-before-run` and `node-linker=pnp` will now print a warning.

deps/status/src/checkDepsStatus.ts

@@ -50,6 +50,7 @@ export type CheckDepsStatusOptions = Pick<Config,
 | 'excludeLinksFromLockfile'
 | 'injectWorkspacePackages'
 | 'linkWorkspacePackages'
+| 'nodeLinker'
 | 'hooks'
 | 'peersSuffixMaxLength'
 | 'rootProjectManifest'
@@ -109,12 +110,18 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
     catalogs,
     excludeLinksFromLockfile,
     linkWorkspacePackages,
+    nodeLinker,
     rootProjectManifest,
     rootProjectManifestDir,
     sharedWorkspaceLockfile,
     workspaceDir,
   } = opts
 
+  if (nodeLinker === 'pnp') {
+    globalWarn('verify-deps-before-run does not work with node-linker=pnp')
+    return { upToDate: true, workspaceState: undefined }
+  }
+
   const rootManifestOptions = rootProjectManifest && rootProjectManifestDir
     ? getOptionsFromRootManifest(rootProjectManifestDir, rootProjectManifest)
     : undefined
@@ -171,8 +178,18 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
       }
     }
 
+    let statModulesDir: (project: Project) => Promise<fs.Stats | undefined>
+    if (nodeLinker === 'hoisted') {
+      const statsPromise = safeStat(path.join(rootProjectManifestDir, 'node_modules'))
+      statModulesDir = () => statsPromise
+    } else {
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      const _nodeLinkerTypeGuard: 'isolated' | undefined = nodeLinker // static type assertion
+      statModulesDir = project => safeStat(path.join(project.rootDir, 'node_modules'))
+    }
+
     const allManifestStats = await Promise.all(allProjects.map(async project => {
-      const modulesDirStatsPromise = safeStat(path.join(project.rootDir, 'node_modules'))
+      const modulesDirStatsPromise = statModulesDir(project)
       const manifestStats = await statManifestFile(project.rootDir)
       if (!manifestStats) {
         // this error should not happen

pnpm/test/verifyDepsBeforeRun/issue-9424.ts

@@ -0,0 +1,74 @@
+import fs from 'fs'
+import path from 'path'
+import { preparePackages } from '@pnpm/prepare'
+import { type ProjectManifest } from '@pnpm/types'
+import { type WorkspaceState, loadWorkspaceState } from '@pnpm/workspace.state'
+import { sync as writeYamlFile } from 'write-yaml-file'
+import { execPnpm, execPnpmSync } from '../utils'
+
+test('hoisted node linker and node_modules not exist (#9424)', async () => {
+  const config = [
+    '--config.verify-deps-before-run=error',
+    '--config.node-linker=hoisted',
+  ] as const
+
+  type PackageName = 'has-deps' | 'has-no-deps'
+  const manifests: Record<PackageName, ProjectManifest> = {
+    'has-deps': {
+      name: 'has-deps',
+      private: true,
+      dependencies: {
+        '@pnpm.e2e/foo': '=100.0.0',
+      },
+      scripts: {
+        start: 'echo hello from has-deps',
+      },
+    },
+    'has-no-deps': {
+      name: 'has-no-deps',
+      private: true,
+      scripts: {
+        start: 'echo hello from has-no-deps',
+      },
+    },
+  }
+
+  preparePackages([manifests['has-deps'], manifests['has-no-deps']])
+
+  writeYamlFile('pnpm-workspace.yaml', { packages: ['**', '!store/**'] })
+
+  // attempting to execute a script recursively without installing dependencies should fail
+  {
+    const { status, stdout } = execPnpmSync([...config, '--recursive', 'start'])
+    expect(status).not.toBe(0)
+    expect(stdout.toString()).toContain('Cannot check whether dependencies are outdated')
+  }
+
+  await execPnpm([...config, 'install'])
+
+  // pnpm install should create a packages list cache
+  expect(loadWorkspaceState(process.cwd())).toMatchObject({
+    lastValidatedTimestamp: expect.any(Number),
+    pnpmfileExists: false,
+    filteredInstall: false,
+    projects: {
+      [path.resolve('has-deps')]: { name: 'has-deps', version: '0.0.0' },
+      [path.resolve('has-no-deps')]: { name: 'has-no-deps', version: '0.0.0' },
+    },
+    settings: {
+      nodeLinker: 'hoisted',
+    },
+  } as Partial<WorkspaceState>)
+
+  // pnpm install creates a node_modules at root, but none in the workspace members
+  expect(fs.readdirSync(process.cwd())).toContain('node_modules')
+  expect(fs.readdirSync(path.resolve('has-deps'))).not.toContain('node_modules')
+  expect(fs.readdirSync(path.resolve('has-no-deps'))).not.toContain('node_modules')
+
+  // should be able to execute a script recursively after dependencies have been installed
+  {
+    const { stdout } = execPnpmSync([...config, '--recursive', 'start'], { expectSuccess: true })
+    expect(stdout.toString()).toContain('hello from has-deps')
+    expect(stdout.toString()).toContain('hello from has-no-deps')
+  }
+})