refactor: use Promise.withResolvers in node-only flows and tests (#2378)

Author: trivikr

cli/src/cli.ts

@@ -14,20 +14,21 @@ const DEFAULT_FRONTEND_URL = 'https://npmx.dev/'
 const DEV_FRONTEND_URL = 'http://127.0.0.1:3000/'
 
 async function runNpmLogin(): Promise<boolean> {
-  return new Promise(resolve => {
-    const child = spawn('npm', ['login', `--registry=${NPM_REGISTRY_URL}`], {
-      stdio: 'inherit',
-      shell: true,
-    })
+  const { promise, resolve } = Promise.withResolvers<boolean>()
+  const child = spawn('npm', ['login', `--registry=${NPM_REGISTRY_URL}`], {
+    stdio: 'inherit',
+    shell: true,
+  })
 
-    child.on('close', code => {
-      resolve(code === 0)
-    })
+  child.on('close', code => {
+    resolve(code === 0)
+  })
 
-    child.on('error', () => {
-      resolve(false)
-    })
+  child.on('error', () => {
+    resolve(false)
   })
+
+  return promise
 }
 
 const main = defineCommand({

cli/src/npm-client.ts

@@ -179,139 +179,140 @@ async function execNpmInteractive(
   options: ExecNpmOptions = {},
 ): Promise<NpmExecResult> {
   const openUrls = options.openUrls === true
+  const { promise, resolve } = Promise.withResolvers<NpmExecResult>()
 
   // Lazy-load node-pty so the native addon is only required when interactive mode is actually used.
   const pty = await import('@lydell/node-pty')
 
-  return new Promise(resolve => {
-    const npmArgs = options.otp ? [...args, '--otp', options.otp] : args
+  const npmArgs = options.otp ? [...args, '--otp', options.otp] : args
 
-    if (!options.silent) {
-      const displayCmd = options.otp
-        ? ['npm', ...args, '--otp', '******'].join(' ')
-        : ['npm', ...args].join(' ')
-      logCommand(`${displayCmd} (interactive/pty)`)
-    }
+  if (!options.silent) {
+    const displayCmd = options.otp
+      ? ['npm', ...args, '--otp', '******'].join(' ')
+      : ['npm', ...args].join(' ')
+    logCommand(`${displayCmd} (interactive/pty)`)
+  }
 
-    let output = ''
-    let resolved = false
-    let otpPromptSeen = false
-    let authUrlSeen = false
-    let enterSent = false
-    let authUrlTimeout: ReturnType<typeof setTimeout> | null = null
-    let authUrlTimedOut = false
+  let output = ''
+  let resolved = false
+  let otpPromptSeen = false
+  let authUrlSeen = false
+  let enterSent = false
+  let authUrlTimeout: ReturnType<typeof setTimeout> | null = null
+  let authUrlTimedOut = false
 
-    const env = createNpmEnv()
+  const env = createNpmEnv()
 
-    // When openUrls is false, tell npm not to open the browser.
-    // npm still prints the auth URL and polls doneUrl
-    if (!openUrls) {
-      env.npm_config_browser = 'false'
-    }
+  // When openUrls is false, tell npm not to open the browser.
+  // npm still prints the auth URL and polls doneUrl
+  if (!openUrls) {
+    env.npm_config_browser = 'false'
+  }
 
-    const child = pty.spawn('npm', npmArgs, {
-      name: 'xterm-256color',
-      cols: 120,
-      rows: 30,
-      env,
-    })
+  const child = pty.spawn('npm', npmArgs, {
+    name: 'xterm-256color',
+    cols: 120,
+    rows: 30,
+    env,
+  })
 
-    // General timeout: 5 minutes (covers non-auth interactive commands)
-    const timeout = setTimeout(() => {
-      if (resolved) return
-      logDebug('Interactive command timed out', { output })
-      child.kill()
-    }, 300000)
-
-    child.onData((data: string) => {
-      output += data
-      const clean = stripAnsi(data)
-      logDebug('pty data:', { text: clean.trim() })
-
-      const cleanAll = stripAnsi(output)
-
-      // Detect auth URL in output and notify the caller.
-      if (!authUrlSeen) {
-        const urlMatch = cleanAll.match(AUTH_URL_TITLE_RE)
-
-        if (urlMatch && urlMatch[1]) {
-          authUrlSeen = true
-          const authUrl = urlMatch[1].replace(/[.,;:!?)]+$/, '')
-          logDebug('Auth URL detected:', { authUrl, openUrls })
-          options.onAuthUrl?.(authUrl)
-
-          authUrlTimeout = setTimeout(() => {
-            if (resolved) return
-            authUrlTimedOut = true
-            logDebug('Auth URL timeout (90s) — killing process')
-            logError('Authentication timed out after 90 seconds')
-            child.kill()
-          }, AUTH_URL_TIMEOUT_MS)
-        }
+  // General timeout: 5 minutes (covers non-auth interactive commands)
+  const timeout = setTimeout(() => {
+    if (resolved) return
+    logDebug('Interactive command timed out', { output })
+    child.kill()
+  }, 300000)
+
+  child.onData((data: string) => {
+    output += data
+    const clean = stripAnsi(data)
+    logDebug('pty data:', { text: clean.trim() })
+
+    const cleanAll = stripAnsi(output)
+
+    // Detect auth URL in output and notify the caller.
+    if (!authUrlSeen) {
+      const urlMatch = cleanAll.match(AUTH_URL_TITLE_RE)
+
+      if (urlMatch && urlMatch[1]) {
+        authUrlSeen = true
+        const authUrl = urlMatch[1].replace(/[.,;:!?)]+$/, '')
+        logDebug('Auth URL detected:', { authUrl, openUrls })
+        options.onAuthUrl?.(authUrl)
+
+        authUrlTimeout = setTimeout(() => {
+          if (resolved) return
+          authUrlTimedOut = true
+          logDebug('Auth URL timeout (90s) — killing process')
+          logError('Authentication timed out after 90 seconds')
+          child.kill()
+        }, AUTH_URL_TIMEOUT_MS)
       }
+    }
 
-      if (authUrlSeen && openUrls && !enterSent && AUTH_URL_PROMPT_RE.test(cleanAll)) {
-        enterSent = true
-        logDebug('Web auth prompt detected, pressing ENTER')
-        child.write('\r')
-      }
+    if (authUrlSeen && openUrls && !enterSent && AUTH_URL_PROMPT_RE.test(cleanAll)) {
+      enterSent = true
+      logDebug('Web auth prompt detected, pressing ENTER')
+      child.write('\r')
+    }
 
-      if (!otpPromptSeen && OTP_PROMPT_RE.test(cleanAll)) {
-        otpPromptSeen = true
-        if (options.otp) {
-          logDebug('OTP prompt detected, writing OTP')
-          child.write(options.otp + '\r')
-        } else {
-          logDebug('OTP prompt detected but no OTP provided, killing process')
-          child.kill()
-        }
+    if (!otpPromptSeen && OTP_PROMPT_RE.test(cleanAll)) {
+      otpPromptSeen = true
+      if (options.otp) {
+        logDebug('OTP prompt detected, writing OTP')
+        child.write(options.otp + '\r')
+      } else {
+        logDebug('OTP prompt detected but no OTP provided, killing process')
+        child.kill()
       }
-    })
+    }
+  })
+
+  child.onExit(({ exitCode }) => {
+    if (resolved) return
+    resolved = true
+    clearTimeout(timeout)
+    if (authUrlTimeout) clearTimeout(authUrlTimeout)
+
+    const cleanOutput = stripAnsi(output)
+    logDebug('Interactive command exited:', { exitCode, output: cleanOutput })
+
+    const requiresOtp =
+      authUrlTimedOut || (otpPromptSeen && !options.otp) || detectOtpRequired(cleanOutput)
+    const authFailure = detectAuthFailure(cleanOutput)
+    const urls = extractUrls(cleanOutput)
 
-    child.onExit(({ exitCode }) => {
-      if (resolved) return
-      resolved = true
-      clearTimeout(timeout)
-      if (authUrlTimeout) clearTimeout(authUrlTimeout)
-
-      const cleanOutput = stripAnsi(output)
-      logDebug('Interactive command exited:', { exitCode, output: cleanOutput })
-
-      const requiresOtp =
-        authUrlTimedOut || (otpPromptSeen && !options.otp) || detectOtpRequired(cleanOutput)
-      const authFailure = detectAuthFailure(cleanOutput)
-      const urls = extractUrls(cleanOutput)
-
-      if (!options.silent) {
-        if (exitCode === 0) {
-          logSuccess('Done')
-        } else if (requiresOtp) {
-          logError('OTP required')
-        } else if (authFailure) {
-          logError('Authentication required - please run "npm login" and restart the connector')
-        } else {
-          const firstLine = filterNpmWarnings(cleanOutput).split('\n')[0] || 'Command failed'
-          logError(firstLine)
-        }
+    if (!options.silent) {
+      if (exitCode === 0) {
+        logSuccess('Done')
+      } else if (requiresOtp) {
+        logError('OTP required')
+      } else if (authFailure) {
+        logError('Authentication required - please run "npm login" and restart the connector')
+      } else {
+        const firstLine = filterNpmWarnings(cleanOutput).split('\n')[0] || 'Command failed'
+        logError(firstLine)
       }
+    }
+
+    // If auth URL timed out, force a non-zero exit code so it's marked as failed
+    const finalExitCode = authUrlTimedOut ? 1 : exitCode
 
-      // If auth URL timed out, force a non-zero exit code so it's marked as failed
-      const finalExitCode = authUrlTimedOut ? 1 : exitCode
-
-      resolve({
-        stdout: cleanOutput.trim(),
-        stderr: requiresOtp
-          ? 'This operation requires a one-time password (OTP).'
-          : authFailure
-            ? 'Authentication failed. Please run "npm login" and restart the connector.'
-            : filterNpmWarnings(cleanOutput),
-        exitCode: finalExitCode,
-        requiresOtp,
-        authFailure,
-        urls: urls.length > 0 ? urls : undefined,
-      })
+    resolve({
+      stdout: cleanOutput.trim(),
+      stderr: requiresOtp
+        ? 'This operation requires a one-time password (OTP).'
+        : authFailure
+          ? 'Authentication failed. Please run "npm login" and restart the connector.'
+          : filterNpmWarnings(cleanOutput),
+      exitCode: finalExitCode,
+      requiresOtp,
+      authFailure,
+      urls: urls.length > 0 ? urls : undefined,
     })
   })
+
+  return promise
 }
 
 async function execNpm(args: string[], options: ExecNpmOptions = {}): Promise<NpmExecResult> {

test/nuxt/components/Package/Versions.spec.ts

@@ -815,11 +815,8 @@ describe('PackageVersions', () => {
   describe('loading states', () => {
     it('shows loading spinner when fetching versions', async () => {
       // Create a promise that won't resolve immediately
-      let resolvePromise: (value: unknown[]) => void
-      const loadingPromise = new Promise<unknown[]>(resolve => {
-        resolvePromise = resolve
-      })
-      mockFetchAllPackageVersions.mockReturnValue(loadingPromise)
+      const { promise, resolve } = Promise.withResolvers<unknown[]>()
+      mockFetchAllPackageVersions.mockReturnValue(promise)
 
       const component = await mountSuspended(PackageVersions, {
         props: {
@@ -842,14 +839,12 @@ describe('PackageVersions', () => {
       })
 
       // Resolve the promise to clean up
-      resolvePromise!([])
+      resolve([])
     })
 
     it('shows loading spinner for other versions when fetching', async () => {
-      let resolvePromise: (value: unknown[]) => void
-      const loadingPromise = new Promise<unknown[]>(resolve => {
-        resolvePromise = resolve
-      })
+      const { promise: loadingPromise, resolve: resolvePromise } =
+        Promise.withResolvers<unknown[]>()
       mockFetchAllPackageVersions.mockReturnValue(loadingPromise)
 
       const component = await mountSuspended(PackageVersions, {
@@ -876,7 +871,7 @@ describe('PackageVersions', () => {
       })
 
       // Resolve the promise to clean up
-      resolvePromise!([])
+      resolvePromise([])
     })
   })
 

test/nuxt/components/VersionSelector.spec.ts

@@ -519,11 +519,8 @@ describe('VersionSelector', () => {
 
   describe('loading states', () => {
     it('shows loading spinner when fetching versions', async () => {
-      let resolvePromise: (value: unknown[]) => void
-      const loadingPromise = new Promise<unknown[]>(resolve => {
-        resolvePromise = resolve
-      })
-      mockFetchAllPackageVersions.mockReturnValue(loadingPromise)
+      const { promise, resolve } = Promise.withResolvers<unknown[]>()
+      mockFetchAllPackageVersions.mockReturnValue(promise)
 
       const component = await mountSuspended(VersionSelector, {
         props: {
@@ -549,7 +546,7 @@ describe('VersionSelector', () => {
       })
 
       // Resolve the promise to clean up
-      resolvePromise!([])
+      resolve([])
     })
   })