fix: check for delete modules dir in sub-project (#8967)

* fix: check for delete modules dir in sub-project

Fixes https://github.com/pnpm/pnpm/issues/8959

* docs: remove todo

* refactor: merge branching

* docs: explain

* fix: actually return `upToDate: false`

* test: issue 8959

* test: add assertions to `multiProjectWorkspace.ts`

* feat: delete `optionalDependencies`

* fix: filtered install

.changeset/lucky-scissors-guess.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/deps.status": patch
+"pnpm": patch
+---
+
+Make sure that the deletion of a `node_modules` in a sub-project of a monorepo is detected as out-of-date [#8959](https://github.com/pnpm/pnpm/issues/8959).

deps/status/src/checkDepsStatus.ts

@@ -157,14 +157,34 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions): Promise<{ upToDa
     }
 
     const allManifestStats = await Promise.all(allProjects.map(async project => {
+      const modulesDirStatsPromise = safeStat(path.join(project.rootDir, 'node_modules'))
       const manifestStats = await statManifestFile(project.rootDir)
       if (!manifestStats) {
         // this error should not happen
         throw new Error(`Cannot find one of ${MANIFEST_BASE_NAMES.join(', ')} in ${project.rootDir}`)
       }
-      return { project, manifestStats }
+      return {
+        project,
+        manifestStats,
+        modulesDirStats: await modulesDirStatsPromise,
+      }
     }))
 
+    if (!workspaceState.filteredInstall) {
+      for (const { modulesDirStats, project } of allManifestStats) {
+        if (modulesDirStats) continue
+        if (isEmpty({
+          ...project.manifest.dependencies,
+          ...project.manifest.devDependencies,
+        })) continue
+        const id = project.manifest.name ?? project.rootDir
+        return {
+          upToDate: false,
+          issue: `Workspace package ${id} has dependencies but does not have a modules directory`,
+        }
+      }
+    }
+
     const modifiedProjects = allManifestStats.filter(
       ({ manifestStats }) =>
         manifestStats.mtime.valueOf() > workspaceState.lastValidatedTimestamp

pnpm/test/install/issue-8959.ts

@@ -0,0 +1,40 @@
+import fs from 'fs'
+import { preparePackages } from '@pnpm/prepare'
+import { sync as writeYamlFile } from 'write-yaml-file'
+import { execPnpm } from '../utils'
+
+// Covers https://github.com/pnpm/pnpm/issues/8959
+test('restores deleted modules dir of a workspace package', async () => {
+  preparePackages([
+    {
+      location: '.',
+      package: {
+        name: 'root',
+        version: '0.0.0',
+        private: true,
+      },
+    },
+    {
+      location: 'packages/foo',
+      package: {
+        name: 'foo',
+        version: '0.0.0',
+        private: true,
+        dependencies: {
+          'is-positive': '1.0.0',
+        },
+      },
+    },
+  ])
+
+  writeYamlFile('pnpm-workspace.yaml', { packages: ['packages/*'] })
+
+  await execPnpm(['install'])
+  expect(fs.readdirSync('node_modules')).toContain('.pnpm-workspace-state.json')
+  expect(fs.readdirSync('packages/foo/node_modules')).toContain('is-positive')
+
+  fs.rmSync('packages/foo/node_modules', { recursive: true })
+  await execPnpm(['--reporter=append-only', 'install'])
+
+  expect(fs.readdirSync('packages/foo/node_modules')).toContain('is-positive')
+})

pnpm/test/verifyDepsBeforeRun/multiProjectWorkspace.ts

@@ -278,6 +278,23 @@ test('single dependency', async () => {
     expect(stdout.toString()).toContain('hello from root')
   }
 
+  fs.rmSync('foo/node_modules', { recursive: true })
+
+  // attempting to execute a script after the modules directory of a workspace package has been deleted should fail
+  {
+    const { status, stdout } = execPnpmSync([...CONFIG, 'start'])
+    expect(status).not.toBe(0)
+    expect(stdout.toString()).toContain('Workspace package foo has dependencies but does not have a modules directory')
+  }
+
+  await execPnpm([...CONFIG, 'install'])
+
+  // should be able to execute a script after dependencies have been updated
+  {
+    const { stdout } = execPnpmSync([...CONFIG, 'start'], { expectSuccess: true })
+    expect(stdout.toString()).toContain('hello from root')
+  }
+
   // should set env.npm_config_verify_deps_before_run to false for all the scripts (to skip check in nested scripts)
   await execPnpm([...CONFIG, '--recursive', 'run', 'checkEnv'])
 })