fix: remove warnings after approve no builds (#9307)

close #9296

---------

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

.changeset/breezy-bananas-lay.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/modules-yaml": minor
+"@pnpm/exec.build-commands": patch
+"pnpm": patch
+---
+
+Remove warnings after having explicitly approved no builds [#9296](https://github.com/pnpm/pnpm/issues/9296).

.changeset/nine-cooks-open.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/modules-yaml": minor
+---
+
+Add type `StrictModules`.

exec/build-commands/src/approveBuilds.ts

@@ -1,5 +1,6 @@
 import { type Config } from '@pnpm/config'
 import { globalInfo } from '@pnpm/logger'
+import { type StrictModules, writeModulesManifest } from '@pnpm/modules-yaml'
 import { tryReadProjectManifest } from '@pnpm/read-project-manifest'
 import { lexCompare } from '@pnpm/util.lex-comparator'
 import renderHelp from 'render-help'
@@ -44,7 +45,11 @@ export function rcOptionsTypes (): Record<string, unknown> {
 }
 
 export async function handler (opts: ApproveBuildsCommandOpts & RebuildCommandOpts): Promise<void> {
-  const automaticallyIgnoredBuilds = await getAutomaticallyIgnoredBuilds(opts)
+  const {
+    automaticallyIgnoredBuilds,
+    modulesDir,
+    modulesManifest,
+  } = await getAutomaticallyIgnoredBuilds(opts)
   if (!automaticallyIgnoredBuilds?.length) {
     globalInfo('There are no packages awaiting approval')
     return
@@ -143,6 +148,9 @@ Do you approve?`,
       ...opts,
       onlyBuiltDependencies: updatedOnlyBuiltDependencies,
     }, buildPackages)
+  } else if (modulesManifest) {
+    delete modulesManifest.ignoredBuilds
+    await writeModulesManifest(modulesDir, modulesManifest as StrictModules)
   }
 }
 

exec/build-commands/src/getAutomaticallyIgnoredBuilds.ts

@@ -1,11 +1,23 @@
 import path from 'path'
-import { readModulesManifest } from '@pnpm/modules-yaml'
+import { type Modules, readModulesManifest } from '@pnpm/modules-yaml'
 import { type IgnoredBuildsCommandOpts } from './ignoredBuilds'
 
-export async function getAutomaticallyIgnoredBuilds (opts: IgnoredBuildsCommandOpts): Promise<null | string[]> {
-  const modulesManifest = await readModulesManifest(opts.modulesDir ?? path.join(opts.lockfileDir ?? opts.dir, 'node_modules'))
-  if (modulesManifest == null) {
-    return null
-  }
-  return modulesManifest?.ignoredBuilds ?? []
+export interface GetAutomaticallyIgnoredBuildsResult {
+  automaticallyIgnoredBuilds: string[] | null
+  modulesDir: string
+  modulesManifest: Modules | null
+}
+
+export async function getAutomaticallyIgnoredBuilds (opts: IgnoredBuildsCommandOpts): Promise<GetAutomaticallyIgnoredBuildsResult> {
+  const modulesDir = getModulesDir(opts)
+  const modulesManifest = await readModulesManifest(modulesDir)
+  return {
+    automaticallyIgnoredBuilds: modulesManifest && (modulesManifest.ignoredBuilds ?? []),
+    modulesDir,
+    modulesManifest,
+  }
+}
+
+function getModulesDir (opts: IgnoredBuildsCommandOpts): string {
+  return opts.modulesDir ?? path.join(opts.lockfileDir ?? opts.dir, 'node_modules')
 }

exec/build-commands/src/ignoredBuilds.ts

@@ -23,7 +23,11 @@ export function rcOptionsTypes (): Record<string, unknown> {
 
 export async function handler (opts: IgnoredBuildsCommandOpts): Promise<string> {
   const ignoredBuiltDependencies = opts.rootProjectManifest?.pnpm?.ignoredBuiltDependencies ?? []
-  const automaticallyIgnoredBuilds = (await getAutomaticallyIgnoredBuilds(opts))?.filter((automaticallyIgnoredBuild) => !ignoredBuiltDependencies.includes(automaticallyIgnoredBuild))
+  let { automaticallyIgnoredBuilds } = await getAutomaticallyIgnoredBuilds(opts)
+  if (automaticallyIgnoredBuilds) {
+    automaticallyIgnoredBuilds = automaticallyIgnoredBuilds
+      .filter((automaticallyIgnoredBuild) => !ignoredBuiltDependencies.includes(automaticallyIgnoredBuild))
+  }
   let output = 'Automatically ignored builds during installation:\n'
   if (automaticallyIgnoredBuilds == null) {
     output += '  Cannot identify as no node_modules found'

exec/build-commands/test/approveBuilds.test.ts

@@ -3,9 +3,11 @@ import path from 'path'
 import * as enquirer from 'enquirer'
 import { approveBuilds } from '@pnpm/exec.build-commands'
 import { install } from '@pnpm/plugin-commands-installation'
+import { type RebuildCommandOpts } from '@pnpm/plugin-commands-rebuild'
 import { prepare } from '@pnpm/prepare'
 import { type ProjectManifest } from '@pnpm/types'
 import { getConfig } from '@pnpm/config'
+import { type Modules, readModulesManifest } from '@pnpm/modules-yaml'
 import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
 import { sync as loadJsonFile } from 'load-json-file'
 import omit from 'ramda/src/omit'
@@ -19,7 +21,9 @@ jest.mock('enquirer', () => ({ prompt: jest.fn() }))
 // eslint-disable-next-line
 const prompt = enquirer.prompt as any
 
-const runApproveBuilds = async (opts = {}) => {
+type ApproveBuildsOptions = Partial<approveBuilds.ApproveBuildsCommandOpts & RebuildCommandOpts>
+
+async function approveSomeBuilds (opts?: ApproveBuildsOptions) {
   const cliOptions = {
     argv: [],
     dir: process.cwd(),
@@ -49,6 +53,29 @@ const runApproveBuilds = async (opts = {}) => {
   await approveBuilds.handler({ ...config, ...opts })
 }
 
+async function approveNoBuilds (opts?: ApproveBuildsOptions) {
+  const cliOptions = {
+    argv: [],
+    dir: process.cwd(),
+    registry: `http://localhost:${REGISTRY_MOCK_PORT}`,
+  }
+  const config = {
+    ...omit(['reporter'], (await getConfig({
+      cliOptions,
+      packageManager: { name: 'pnpm', version: '' },
+    })).config),
+    storeDir: path.resolve('store'),
+    cacheDir: path.resolve('cache'),
+  }
+  await install.handler({ ...config, argv: { original: [] } })
+
+  prompt.mockResolvedValueOnce({
+    result: [],
+  })
+
+  await approveBuilds.handler({ ...config, ...opts })
+}
+
 test('approve selected build', async () => {
   prepare({
     dependencies: {
@@ -57,7 +84,7 @@ test('approve selected build', async () => {
     },
   })
 
-  await runApproveBuilds()
+  await approveSomeBuilds()
 
   const manifest = loadJsonFile<ProjectManifest>(path.resolve('package.json'))
   expect(manifest.pnpm?.onlyBuiltDependencies).toStrictEqual(['@pnpm.e2e/pre-and-postinstall-scripts-example'])
@@ -68,6 +95,31 @@ test('approve selected build', async () => {
   expect(fs.existsSync('node_modules/@pnpm.e2e/install-script-example/generated-by-install.js')).toBeFalsy()
 })
 
+test('approve no builds', async () => {
+  prepare({
+    dependencies: {
+      '@pnpm.e2e/pre-and-postinstall-scripts-example': '1.0.0',
+      '@pnpm.e2e/install-script-example': '*',
+    },
+  })
+
+  await approveNoBuilds()
+
+  const manifest = loadJsonFile<ProjectManifest>(path.resolve('package.json'))
+  expect(manifest.pnpm?.onlyBuiltDependencies).toBeUndefined()
+  expect(manifest.pnpm?.ignoredBuiltDependencies?.sort()).toStrictEqual([
+    '@pnpm.e2e/install-script-example',
+    '@pnpm.e2e/pre-and-postinstall-scripts-example',
+  ])
+
+  expect(fs.readdirSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example')).not.toContain('generated-by-preinstall.js')
+  expect(fs.readdirSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example')).not.toContain('generated-by-postinstall.js')
+  expect(fs.readdirSync('node_modules/@pnpm.e2e/install-script-example')).not.toContain('generated-by-install.js')
+
+  // Covers https://github.com/pnpm/pnpm/issues/9296
+  expect(await readModulesManifest('node_modules')).not.toHaveProperty(['ignoredBuilds' satisfies keyof Modules])
+})
+
 test("works when root project manifest doesn't exist in a workspace", async () => {
   tempDir()
 
@@ -82,7 +134,7 @@ test("works when root project manifest doesn't exist in a workspace", async () =
   const workspaceManifestFile = path.join(workspaceDir, 'pnpm-workspace.yaml')
   writeYamlFile(workspaceManifestFile, { packages: ['packages/*'] })
   process.chdir('workspace/packages/project')
-  await runApproveBuilds({ workspaceDir, rootProjectManifestDir: workspaceDir })
+  await approveSomeBuilds({ workspaceDir, rootProjectManifestDir: workspaceDir })
 
   expect(readYamlFile(workspaceManifestFile)).toStrictEqual({
     packages: ['packages/*'],
@@ -108,7 +160,7 @@ test('should update onlyBuiltDependencies when package.json exists with ignoredB
 
   const workspaceManifestFile = path.join(temp, 'pnpm-workspace.yaml')
   writeYamlFile(workspaceManifestFile, { packages: ['packages/*'] })
-  await runApproveBuilds({ workspaceDir: temp, rootProjectManifestDir: temp })
+  await approveSomeBuilds({ workspaceDir: temp, rootProjectManifestDir: temp })
 
   expect(readYamlFile(workspaceManifestFile)).toStrictEqual({
     packages: ['packages/*'],
@@ -136,7 +188,7 @@ test('should approve builds when package.json exists with onlyBuiltDependencies
 
   const workspaceManifestFile = path.join(temp, 'pnpm-workspace.yaml')
   writeYamlFile(workspaceManifestFile, { packages: ['packages/*'] })
-  await runApproveBuilds({ workspaceDir: temp, rootProjectManifestDir: temp })
+  await approveSomeBuilds({ workspaceDir: temp, rootProjectManifestDir: temp })
 
   expect(readYamlFile(workspaceManifestFile)).toStrictEqual({
     packages: ['packages/*'],
@@ -160,7 +212,7 @@ test('should approve builds with package.json that has no onlyBuiltDependencies
 
   const workspaceManifestFile = path.join(temp, 'pnpm-workspace.yaml')
   writeYamlFile(workspaceManifestFile, { packages: ['packages/*'] })
-  await runApproveBuilds({ workspaceDir: temp, rootProjectManifestDir: temp })
+  await approveSomeBuilds({ workspaceDir: temp, rootProjectManifestDir: temp })
 
   expect(readYamlFile(workspaceManifestFile)).toStrictEqual({
     packages: ['packages/*'],

pkg-manager/modules-yaml/src/index.ts

@@ -95,9 +95,13 @@ const YAML_OPTS = {
   sortKeys: true,
 }
 
+export interface StrictModules extends Modules {
+  registries: Registries
+}
+
 export async function writeModulesManifest (
   modulesDir: string,
-  modules: Modules & { registries: Registries },
+  modules: StrictModules,
   opts?: {
     makeModulesDir?: boolean
   }