refactor: fix types and eslint errors in utils/shell.ts

Author: serhalp

eslint_temporary_suppressions.js

@@ -1071,19 +1071,6 @@ export default [
       '@typescript-eslint/prefer-nullish-coalescing': 'off',
     },
   },
-  {
-    files: ['src/utils/shell.ts'],
-    rules: {
-      '@typescript-eslint/no-unsafe-return': 'off',
-      '@typescript-eslint/no-unsafe-call': 'off',
-      'n/no-process-exit': 'off',
-      '@typescript-eslint/no-explicit-any': 'off',
-      '@typescript-eslint/no-unsafe-argument': 'off',
-      '@typescript-eslint/no-unsafe-member-access': 'off',
-      '@typescript-eslint/no-floating-promises': 'off',
-      '@typescript-eslint/restrict-template-expressions': 'off',
-    },
-  },
   {
     files: ['src/utils/sign-redirect.ts'],
     rules: {

src/utils/shell.ts

@@ -12,29 +12,25 @@ import { processOnExit } from './dev.js'
 const isErrnoException = (value: unknown): value is NodeJS.ErrnoException =>
   value instanceof Error && Object.hasOwn(value, 'code')
 
-const createStripAnsiControlCharsStream = (): Transform => new Transform({
-  transform(chunk, _encoding, callback) {
-    const text = typeof chunk === 'string' ? chunk : chunk.toString()
-    callback(null, stripVTControlCharacters(text))
-  }
-})
+const createStripAnsiControlCharsStream = (): Transform =>
+  new Transform({
+    transform(chunk, _encoding, callback) {
+      callback(null, stripVTControlCharacters(typeof chunk === 'string' ? chunk : (chunk as unknown)?.toString() ?? ''))
+    },
+  })
 
 const cleanupWork: (() => Promise<void>)[] = []
 
 let cleanupStarted = false
 
-/**
- * @param {object} input
- * @param {number=} input.exitCode The exit code to return when exiting the process after cleanup
- */
 const cleanupBeforeExit = async ({ exitCode }: { exitCode?: number | undefined } = {}) => {
   // If cleanup has started, then wherever started it will be responsible for exiting
   if (!cleanupStarted) {
     cleanupStarted = true
     try {
-      // @ts-expect-error TS(7005) FIXME: Variable 'cleanupWork' implicitly has an 'any[]' t... Remove this comment to see the full error message
       await Promise.all(cleanupWork.map((cleanup) => cleanup()))
     } finally {
+      // eslint-disable-next-line n/no-process-exit
       process.exit(exitCode)
     }
   }
@@ -70,7 +66,7 @@ export const runCommand = (
     // In this case, we want to manually control when to clear and when to render a frame, so we turn this off.
     stopSpinner({ error: false, spinner })
   }
-  const pipeDataWithSpinner = (writeStream: NodeJS.WriteStream, chunk: any) => {
+  const pipeDataWithSpinner = (writeStream: NodeJS.WriteStream, chunk: string | Uint8Array) => {
     if (spinner?.isSpinning) {
       spinner.clear()
     }
@@ -79,14 +75,19 @@ export const runCommand = (
     })
   }
 
-  commandProcess.stdout?.pipe(createStripAnsiControlCharsStream()).on('data', pipeDataWithSpinner.bind(null, process.stdout))
-  commandProcess.stderr?.pipe(createStripAnsiControlCharsStream()).on('data', pipeDataWithSpinner.bind(null, process.stderr))
+  commandProcess.stdout
+    ?.pipe(createStripAnsiControlCharsStream())
+    .on('data', pipeDataWithSpinner.bind(null, process.stdout))
+  commandProcess.stderr
+    ?.pipe(createStripAnsiControlCharsStream())
+    .on('data', pipeDataWithSpinner.bind(null, process.stderr))
   if (commandProcess.stdin != null) {
-    process.stdin?.pipe(commandProcess.stdin)
+    process.stdin.pipe(commandProcess.stdin)
   }
 
   // we can't try->await->catch since we don't want to block on the framework server which
   // is a long running process
+  // eslint-disable-next-line @typescript-eslint/no-floating-promises
   commandProcess.then(async () => {
     const result = await commandProcess
     const [commandWithoutArgs] = command.split(' ')
@@ -98,9 +99,10 @@ export const runCommand = (
       )
     } else {
       const errorMessage = result.failed
-        ? // @ts-expect-error TS(2339) FIXME: Property 'shortMessage' does not exist on type 'Ex... Remove this comment to see the full error message
-          `${NETLIFYDEVERR} ${result.shortMessage}`
-        : `${NETLIFYDEVWARN} "${command}" exited with code ${result.exitCode}`
+        ? // @ts-expect-error FIXME(serhalp): We use `reject: false` which means the resolved value is either the resolved value
+          // or the rejected value, but the types aren't smart enough to know this.
+          `${NETLIFYDEVERR} ${result.shortMessage as string}`
+        : `${NETLIFYDEVWARN} "${command}" exited with code ${result.exitCode.toString()}`
 
       log(`${errorMessage}. Shutting down Netlify Dev server`)
     }
@@ -114,18 +116,10 @@ export const runCommand = (
   return commandProcess
 }
 
-/**
- *
- * @param {object} config
- * @param {string} config.command
- * @param {*} config.error
- * @returns
- */
-// @ts-expect-error TS(7031) FIXME: Binding element 'command' implicitly has an 'any' ... Remove this comment to see the full error message
-const isNonExistingCommandError = ({ command, error: commandError }) => {
+const isNonExistingCommandError = ({ command, error: commandError }: { command: string; error: unknown }) => {
   // `ENOENT` is only returned for non Windows systems
   // See https://github.com/sindresorhus/execa/pull/447
-  if (commandError.code === 'ENOENT') {
+  if (isErrnoException(commandError) && commandError.code === 'ENOENT') {
     return true
   }
 
@@ -136,6 +130,7 @@ const isNonExistingCommandError = ({ command, error: commandError }) => {
 
   // this only works on English versions of Windows
   return (
+    commandError instanceof Error &&
     typeof commandError.message === 'string' &&
     commandError.message.includes('is not recognized as an internal or external command')
   )