Improve type safety in leaf node utility modules

src/utils/deploy/upload-files.ts

@@ -1,24 +1,48 @@
 import fs from 'fs'
 
+import type { NetlifyAPI } from '@netlify/api'
 import backoff from 'backoff'
 import pMap from 'p-map'
 
 import { UPLOAD_INITIAL_DELAY, UPLOAD_MAX_DELAY, UPLOAD_RANDOM_FACTOR } from './constants.js'
+import type { StatusCallback } from './status-cb.js'
+
+export interface UploadFileObj {
+  assetType: 'file' | 'function'
+  body?: string | Buffer | (() => fs.ReadStream)
+  filepath: string
+  invocationMode?: string
+  normalizedPath: string
+  runtime?: string
+  timeout?: number
+}
+
+interface ErrorWithStatus {
+  status: number
+}
 
-// @ts-expect-error TS(7006) FIXME: Parameter 'api' implicitly has an 'any' type.
-const uploadFiles = async (api, deployId, uploadList, { concurrentUpload, maxRetry, statusCb }) => {
-  if (!concurrentUpload || !statusCb || !maxRetry) throw new Error('Missing required option concurrentUpload')
+const isErrorWithStatus = (error: unknown): error is ErrorWithStatus =>
+  typeof error === 'object' &&
+  error !== null &&
+  'status' in error &&
+  typeof (error as { status: unknown }).status === 'number'
+
+const uploadFiles = async (
+  api: NetlifyAPI,
+  deployId: string,
+  uploadList: UploadFileObj[],
+  { concurrentUpload, maxRetry, statusCb }: { concurrentUpload: number; maxRetry: number; statusCb: StatusCallback },
+) => {
   statusCb({
     type: 'upload',
     msg: `Uploading ${uploadList.length} files`,
     phase: 'start',
   })
 
-  // @ts-expect-error TS(7006) FIXME: Parameter 'fileObj' implicitly has an 'any' type.
-  const uploadFile = async (fileObj, index) => {
+  const uploadFile = async (fileObj: UploadFileObj, index: number) => {
     const { assetType, body, filepath, invocationMode, normalizedPath, runtime, timeout } = fileObj
 
-    const readStreamCtor = () => body ?? fs.createReadStream(filepath)
+    const readStreamCtor = () => (body as unknown as fs.ReadStream | undefined) ?? fs.createReadStream(filepath)
 
     statusCb({
       type: 'upload',
@@ -31,7 +55,7 @@ const uploadFiles = async (api, deployId, uploadList, { concurrentUpload, maxRet
         response = await retryUpload(
           () =>
             api.uploadDeployFile({
-              body: readStreamCtor,
+              body: readStreamCtor as unknown as () => fs.ReadStream,
               deployId,
               path: encodeURI(normalizedPath),
             }),
@@ -40,29 +64,23 @@ const uploadFiles = async (api, deployId, uploadList, { concurrentUpload, maxRet
         break
       }
       case 'function': {
-        // @ts-expect-error TS(7006) FIXME: Parameter 'retryCount' implicitly has an 'any' typ... Remove this comment to see the full error message
-        response = await retryUpload((retryCount) => {
+        response = await retryUpload((retryCount: number) => {
           const params = {
-            body: readStreamCtor,
+            body: readStreamCtor as unknown as () => fs.ReadStream,
             deployId,
             invocationMode,
             timeout,
             name: encodeURI(normalizedPath),
             runtime,
-          }
-
-          if (retryCount > 0) {
-            // @ts-expect-error TS(2339) FIXME: Property 'xNfRetryCount' does not exist on type '{... Remove this comment to see the full error message
-            params.xNfRetryCount = retryCount
+            ...(retryCount > 0 && { xNfRetryCount: retryCount }),
           }
 
           return api.uploadDeployFunction(params)
         }, maxRetry)
         break
       }
       default: {
-        const error = new Error('File Object missing assetType property')
-        // @ts-expect-error TS(2339) FIXME: Property 'fileObj' does not exist on type 'Error'.
+        const error = new Error('File Object missing assetType property') as Error & { fileObj: UploadFileObj }
         error.fileObj = fileObj
         throw error
       }
@@ -80,11 +98,9 @@ const uploadFiles = async (api, deployId, uploadList, { concurrentUpload, maxRet
   return results
 }
 
-// @ts-expect-error TS(7006) FIXME: Parameter 'uploadFn' implicitly has an 'any' type.
-const retryUpload = (uploadFn, maxRetry) =>
+const retryUpload = <T>(uploadFn: (retryCount: number) => Promise<T>, maxRetry: number): Promise<T> =>
   new Promise((resolve, reject) => {
-    // @ts-expect-error TS(7034) FIXME: Variable 'lastError' implicitly has type 'any' in ... Remove this comment to see the full error message
-    let lastError
+    let lastError: unknown
 
     const fibonacciBackoff = backoff.fibonacci({
       randomisationFactor: UPLOAD_RANDOM_FACTOR,
@@ -102,18 +118,24 @@ const retryUpload = (uploadFn, maxRetry) =>
         lastError = error
 
         // We don't need to retry for 400 or 422 errors
-        // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
-        if (error.status === 400 || error.status === 422) {
-          reject(error)
-          return
+        if (isErrorWithStatus(error)) {
+          if (error.status === 400 || error.status === 422) {
+            reject(error)
+            return
+          }
+
+          // observed errors: 408, 401 (4** swallowed), 502
+          if (error.status > 400) {
+            fibonacciBackoff.backoff()
+            return
+          }
         }
 
-        // observed errors: 408, 401 (4** swallowed), 502
-        // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
-        if (error.status > 400 || error.name === 'FetchError') {
+        if (error instanceof Error && error.name === 'FetchError') {
           fibonacciBackoff.backoff()
           return
         }
+
         reject(error)
         return
       }
@@ -130,7 +152,6 @@ const retryUpload = (uploadFn, maxRetry) =>
     fibonacciBackoff.on('ready', tryUpload)
 
     fibonacciBackoff.on('fail', () => {
-      // @ts-expect-error TS(7005) FIXME: Variable 'lastError' implicitly has an 'any' type.
       reject(lastError)
     })
 

src/utils/dev.ts

@@ -195,7 +195,6 @@ const getEnvSourceName = (source) => {
 // @ts-expect-error TS(7031) FIXME: Binding element 'devConfig' implicitly has an 'any... Remove this comment to see the full error message
 export const getDotEnvVariables = async ({ devConfig, env, site }): Promise<EnvironmentVariables> => {
   const dotEnvFiles = await loadDotEnvFiles({ envFiles: devConfig.envFiles, projectDir: site.root })
-  // @ts-expect-error TS(2339) FIXME: Property 'env' does not exist on type '{ warning: ... Remove this comment to see the full error message
   dotEnvFiles.forEach(({ env: fileEnv, file }) => {
     const newSourceName = `${file} file`
 

src/utils/dot-env.ts

@@ -7,26 +7,44 @@ import { isFileAsync } from '../lib/fs.js'
 
 import { warn } from './command-helpers.js'
 
-// @ts-expect-error TS(7031) FIXME: Binding element 'envFiles' implicitly has an 'any'... Remove this comment to see the full error message
-export const loadDotEnvFiles = async function ({ envFiles, projectDir }) {
+interface DotEnvFile {
+  file: string
+  env: Record<string, string>
+}
+
+interface DotEnvWarning {
+  warning: string
+}
+
+type DotEnvResult = DotEnvFile | DotEnvWarning
+
+export const loadDotEnvFiles = async function ({
+  envFiles,
+  projectDir,
+}: {
+  envFiles: string[]
+  projectDir: string
+}): Promise<DotEnvFile[]> {
   const response = await tryLoadDotEnvFiles({ projectDir, dotenvFiles: envFiles })
 
-  // @ts-expect-error TS(2532) FIXME: Object is possibly 'undefined'.
-  const filesWithWarning = response.filter((el) => el.warning)
+  const filesWithWarning = response.filter((el): el is DotEnvWarning => 'warning' in el)
   filesWithWarning.forEach((el) => {
-    // @ts-expect-error TS(2532) FIXME: Object is possibly 'undefined'.
     warn(el.warning)
   })
 
-  // @ts-expect-error TS(2532) FIXME: Object is possibly 'undefined'.
-  return response.filter((el) => el.file && el.env)
+  return response.filter((el): el is DotEnvFile => 'file' in el && 'env' in el)
 }
 
 // in the user configuration, the order is highest to lowest
 const defaultEnvFiles = ['.env.development.local', '.env.local', '.env.development', '.env']
 
-// @ts-expect-error TS(7031) FIXME: Binding element 'projectDir' implicitly has an 'an... Remove this comment to see the full error message
-export const tryLoadDotEnvFiles = async ({ dotenvFiles = defaultEnvFiles, projectDir }) => {
+export const tryLoadDotEnvFiles = async ({
+  dotenvFiles = defaultEnvFiles,
+  projectDir,
+}: {
+  dotenvFiles?: string[]
+  projectDir: string
+}): Promise<DotEnvResult[]> => {
   const results = await Promise.all(
     dotenvFiles.map(async (file) => {
       const filepath = path.resolve(projectDir, file)
@@ -37,8 +55,9 @@ export const tryLoadDotEnvFiles = async ({ dotenvFiles = defaultEnvFiles, projec
         }
       } catch (error) {
         return {
-          // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
-          warning: `Failed reading env variables from file: ${filepath}: ${error.message}`,
+          warning: `Failed reading env variables from file: ${filepath}: ${
+            error instanceof Error ? error.message : String(error)
+          }`,
         }
       }
       const content = await readFile(filepath, 'utf-8')
@@ -48,5 +67,5 @@ export const tryLoadDotEnvFiles = async ({ dotenvFiles = defaultEnvFiles, projec
   )
 
   // we return in order of lowest to highest priority
-  return results.filter(Boolean).reverse()
+  return (results.filter(Boolean) as DotEnvResult[]).reverse()
 }

tests/unit/utils/deploy/upload-files.test.ts

@@ -1,7 +1,8 @@
 import { v4 as generateUUID } from 'uuid'
 import { afterAll, expect, test, vi } from 'vitest'
 
-import uploadFiles from '../../../../src/utils/deploy/upload-files.js'
+import type { NetlifyAPI } from '@netlify/api'
+import uploadFiles, { type UploadFileObj } from '../../../../src/utils/deploy/upload-files.js'
 
 vi.mock('../../../../src/utils/deploy/constants.js', async () => {
   const actual = await vi.importActual('../../../../src/utils/deploy/constants.js')
@@ -18,18 +19,17 @@ test('Adds a retry count to function upload requests', async () => {
   const uploadDeployFunction = vi.fn()
   const mockError = new Error('Uh-oh')
 
-  // @ts-expect-error TS(2339) FIXME: Property 'status' does not exist on type 'Error'.
-  mockError.status = 500
+  Object.assign(mockError, { status: 500 })
 
   uploadDeployFunction.mockRejectedValueOnce(mockError)
   uploadDeployFunction.mockRejectedValueOnce(mockError)
   uploadDeployFunction.mockResolvedValueOnce(undefined)
 
   const mockApi = {
     uploadDeployFunction,
-  }
+  } as unknown as NetlifyAPI
   const deployId = generateUUID()
-  const files = [
+  const files: UploadFileObj[] = [
     {
       assetType: 'function',
       filepath: '/some/path/func1.zip',
@@ -55,16 +55,15 @@ test('Does not retry on 400 response from function upload requests', async () =>
   const uploadDeployFunction = vi.fn()
   const mockError = new Error('Uh-oh')
 
-  // @ts-expect-error TS(2339) FIXME: Property 'status' does not exist on type 'Error'.
-  mockError.status = 400
+  Object.assign(mockError, { status: 400 })
 
   uploadDeployFunction.mockRejectedValue(mockError)
 
   const mockApi = {
     uploadDeployFunction,
-  }
+  } as unknown as NetlifyAPI
   const deployId = generateUUID()
-  const files = [
+  const files: UploadFileObj[] = [
     {
       assetType: 'function',
       filepath: '/some/path/func1.zip',