fix(upgrade): take ownership of dependency installation

Author: danielroe

packages/nuxt-cli/src/commands/upgrade.ts

@@ -1,15 +1,18 @@
 import type { PackageJson } from 'pkg-types'
 
+import type { InstallResult } from '../utils/install'
+
 import { existsSync } from 'node:fs'
 import process from 'node:process'
 
-import { cancel, intro, isCancel, note, outro, select, spinner, tasks } from '@clack/prompts'
+import { cancel, intro, isCancel, note, outro, select, spinner } from '@clack/prompts'
 import { defineCommand } from 'citty'
-import { addDependency, dedupeDependencies, detectPackageManager } from 'nypm'
+import { detectPackageManager } from 'nypm'
 import { dirname, relative, resolve } from 'pathe'
 import colors from 'picocolors'
 import { findWorkspaceDir, readPackageJSON } from 'pkg-types'
 
+import { createInstallLog, runDedupe, runInstall, takeUnreportedIgnoredBuilds } from '../utils/install'
 import { loadKit } from '../utils/kit'
 import { logger } from '../utils/logger'
 import { cleanupNuxtDirs, nuxtVersionToGitIdentifier } from '../utils/nuxt'
@@ -188,59 +191,53 @@ export default defineCommand({
 
     const versionType = ctx.args.channel === 'nightly' ? 'nightly' : `latest ${ctx.args.channel}`
 
-    const spin = spinner()
-    spin.start('Upgrading Nuxt')
-
-    await tasks([
-      {
-        title: `Installing ${versionType} Nuxt ${nuxtVersion} release`,
-        task: async () => {
-          await addDependency(npmPackages, {
-            cwd,
-            packageManager,
-            dev: nuxtDependencyType === 'devDependencies',
-            workspace: packageManager?.name === 'pnpm' && existsSync(resolve(cwd, 'pnpm-workspace.yaml')),
-          })
-          return 'Nuxt packages installed'
-        },
-      },
-      ...(method === 'force'
-        ? [{
-            title: `Recreating ${forceRemovals}`,
-            task: async () => {
-              await dedupeDependencies({ recreateLockfile: true })
-              return 'Lockfile recreated'
-            },
-          }]
-        : []),
-      ...(method === 'dedupe'
-        ? [{
-            title: 'Deduping dependencies',
-            task: async () => {
-              await dedupeDependencies()
-              return 'Dependencies deduped'
-            },
-          }]
-        : []),
-      {
-        title: 'Cleaning up build directories',
-        task: async () => {
-          let buildDir: string = '.nuxt'
-          try {
-            const { loadNuxtConfig } = await loadKit(cwd)
-            const nuxtOptions = await loadNuxtConfig({ cwd })
-            buildDir = nuxtOptions.buildDir
-          }
-          catch {
-            // Use default buildDir (.nuxt)
-          }
-          await cleanupNuxtDirs(cwd, buildDir)
-          return 'Build directories cleaned'
-        },
-      },
-    ])
-
-    spin.stop()
+    const verbose = ctx.args.logLevel === 'verbose' || Boolean(process.env.DEBUG)
+
+    const installFailed = await withInstallSpinner(
+      `Installing ${versionType} Nuxt ${nuxtVersion} release`,
+      'Nuxt packages installed',
+      { verbose },
+      hooks => runInstall({
+        cwd,
+        packageManager,
+        dependencies: npmPackages,
+        dev: nuxtDependencyType === 'devDependencies',
+        workspace: packageManager.name === 'pnpm' && existsSync(resolve(cwd, 'pnpm-workspace.yaml')),
+        ...hooks,
+      }),
+    )
+
+    if (installFailed) {
+      process.exit(1)
+    }
+
+    if (method === 'force' || method === 'dedupe') {
+      const recreateLockfile = method === 'force'
+      const failed = await withInstallSpinner(
+        recreateLockfile ? `Recreating ${forceRemovals}` : 'Deduping dependencies',
+        recreateLockfile ? 'Lockfile recreated' : 'Dependencies deduped',
+        { verbose },
+        hooks => runDedupe({ cwd, packageManager, recreateLockfile, ...hooks }),
+      )
+
+      if (failed) {
+        process.exit(1)
+      }
+    }
+
+    const cleanupSpinner = spinner()
+    cleanupSpinner.start('Cleaning up build directories')
+    let buildDir: string = '.nuxt'
+    try {
+      const { loadNuxtConfig } = await loadKit(cwd)
+      const nuxtOptions = await loadNuxtConfig({ cwd })
+      buildDir = nuxtOptions.buildDir
+    }
+    catch {
+      // Use default buildDir (.nuxt)
+    }
+    await cleanupNuxtDirs(cwd, buildDir, { silent: true })
+    cleanupSpinner.stop('Build directories cleaned')
 
     if (method === 'force') {
       logger.info(`If you encounter any issues, revert the changes and try with ${colors.cyan('--no-force')}`)
@@ -276,6 +273,53 @@ export default defineCommand({
   },
 })
 
+interface InstallHooks {
+  onOutput: (line: string) => void
+  onStatus: (message: string) => void
+  signal: AbortSignal
+}
+
+/**
+ * Run a package manager step behind a spinner, surfacing its output only when it
+ * fails (or when running verbosely). Returns whether the step failed.
+ */
+async function withInstallSpinner(
+  title: string,
+  success: string,
+  options: { verbose: boolean },
+  run: (hooks: InstallHooks) => Promise<InstallResult>,
+): Promise<boolean> {
+  const controller = new AbortController()
+  const installLog = createInstallLog({ verbose: options.verbose })
+  const spin = spinner({
+    indicator: 'timer',
+    onCancel: () => controller.abort(),
+  })
+
+  spin.start(title)
+  const result = await run({
+    onOutput: installLog.onOutput,
+    onStatus: message => spin.message(message),
+    signal: controller.signal,
+  })
+
+  if (result.success) {
+    spin.stop(success)
+  }
+  else {
+    spin.error(result.error ?? `${title} failed`)
+  }
+
+  installLog.finish(result)
+
+  const ignoredBuilds = takeUnreportedIgnoredBuilds(result.ignoredBuilds)
+  if (ignoredBuilds.length > 0) {
+    logger.warn(`Build scripts were not run for ${ignoredBuilds.map(name => colors.cyan(name)).join(', ')}.`)
+  }
+
+  return !result.success
+}
+
 // Find which lock file is in use since `nypm.detectPackageManager` doesn't return this
 export function findLockFile(cwd: string, workspaceDir: string, lockFiles: string | Array<string> | undefined) {
   const candidates = typeof lockFiles === 'string' ? [lockFiles] : lockFiles

packages/nuxt-cli/src/utils/install.ts

@@ -8,7 +8,7 @@ import { delimiter, resolve } from 'node:path'
 import process from 'node:process'
 
 import { log, S_BAR } from '@clack/prompts'
-import { addDependency, installDependencies, packageManagers } from 'nypm'
+import { addDependency, dedupeDependencies, installDependencies, packageManagers } from 'nypm'
 import colors from 'picocolors'
 import { provider } from 'std-env'
 import { normalizeSpawnCommand, x } from 'tinyexec'
@@ -107,6 +107,33 @@ export async function runInstall(options: InstallOptions): Promise<InstallResult
   return await execute(command, commandArgs, options)
 }
 
+export interface DedupeOptions extends Omit<InstallOptions, 'dependencies' | 'dev' | 'workspace'> {
+  /** Delete the lockfile and resolve dependencies from scratch. */
+  recreateLockfile?: boolean
+}
+
+/**
+ * Dedupe a project's dependencies, or recreate its lockfile, with the same quiet
+ * output handling as {@link runInstall}.
+ */
+export async function runDedupe(options: DedupeOptions): Promise<InstallResult> {
+  const { exec } = await dedupeDependencies({
+    cwd: options.cwd,
+    packageManager: options.packageManager,
+    recreateLockfile: options.recreateLockfile,
+    dry: true,
+  })
+
+  if (!exec) {
+    return { success: true, output: '', command: '', ignoredBuilds: [] }
+  }
+
+  const args = [...exec.args, ...nonInteractiveArgs(options.packageManager)]
+  const [command, commandArgs] = await withCorepack(exec.command, args)
+
+  return await execute(command, commandArgs, options)
+}
+
 /**
  * Flags that stop a package manager from asking a question we cannot answer, or
  * from failing on something the user can act on after the fact.

packages/nuxt-cli/src/utils/nuxt.ts

@@ -20,8 +20,11 @@ interface NuxtProjectManifest {
   }
 }
 
-export async function cleanupNuxtDirs(rootDir: string, buildDir: string) {
-  logger.info('Cleaning up generated Nuxt files and caches...')
+/** `silent` is for callers that already report progress themselves. */
+export async function cleanupNuxtDirs(rootDir: string, buildDir: string, options: { silent?: boolean } = {}) {
+  if (!options.silent) {
+    logger.info('Cleaning up generated Nuxt files and caches...')
+  }
 
   await rmRecursive(
     [

packages/nuxt-cli/test/unit/utils/install.spec.ts

@@ -1,11 +1,20 @@
+import { existsSync } from 'node:fs'
 import { chmod, mkdtemp, writeFile } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import process from 'node:process'
 
 import { describe, expect, it } from 'vitest'
 
-import { getIgnoredBuilds, isExecutableAvailable, nonInteractiveArgs, runInstall, takeUnreportedIgnoredBuilds } from '../../../src/utils/install'
+import { getIgnoredBuilds, isExecutableAvailable, nonInteractiveArgs, runDedupe, runInstall, takeUnreportedIgnoredBuilds } from '../../../src/utils/install'
+
+async function createFakePackageManager(script: string[] = ['#!/bin/sh', 'echo "all done"']) {
+  const dir = await mkdtemp(join(tmpdir(), 'nuxt-install-test-'))
+  const command = join(dir, 'fake-package-manager')
+  await writeFile(command, script.join('\n'))
+  await chmod(command, 0o755)
+  return { dir, command }
+}
 
 describe('nonInteractiveArgs', () => {
   it('should opt pnpm out of prompts and strict dep builds', () => {
@@ -109,3 +118,46 @@ describe('runInstall', () => {
     expect(result.command).toBe('nuxt-cli-nonexistent-package-manager install')
   })
 })
+
+describe('runDedupe', () => {
+  it.skipIf(process.platform === 'win32')('should dedupe without printing the package manager output', async () => {
+    const { dir, command } = await createFakePackageManager()
+
+    const lines: string[] = []
+    const result = await runDedupe({
+      cwd: dir,
+      packageManager: { name: 'pnpm', command },
+      onOutput: line => lines.push(line),
+    })
+
+    expect(result.success).toBe(true)
+    expect(result.command).toBe(`${command} dedupe --config.confirm-modules-purge=false --config.strict-dep-builds=false`)
+    expect(lines).toEqual(['all done'])
+  })
+
+  it.skipIf(process.platform === 'win32')('should install after removing the lockfile when recreating it', async () => {
+    const { dir, command } = await createFakePackageManager()
+    const lockFile = join(dir, 'pnpm-lock.yaml')
+    await writeFile(lockFile, 'lockfileVersion: 9.0\n')
+
+    const result = await runDedupe({
+      cwd: dir,
+      packageManager: { name: 'pnpm', command, lockFile: 'pnpm-lock.yaml' },
+      recreateLockfile: true,
+    })
+
+    expect(result.success).toBe(true)
+    expect(result.command).toContain(`${command} install`)
+    expect(existsSync(lockFile)).toBe(false)
+  })
+
+  it('should report a missing package manager instead of throwing', async () => {
+    const result = await runDedupe({
+      cwd: process.cwd(),
+      packageManager: { name: 'npm', command: 'nuxt-cli-nonexistent-package-manager' },
+    })
+
+    expect(result.success).toBe(false)
+    expect(result.missingPackageManager).toBe(true)
+  })
+})