Avoid void

.eslintrc.json

@@ -13,8 +13,6 @@
   },
   "reportUnusedDisableDirectives": true,
   "rules": {
-    // Causes a lot of errors - will address separately
-    "@typescript-eslint/no-invalid-void-type": "off",
     "max-lines": ["warn", 500]
   }
 }

lib/__tests__/removeAll.spec.ts

@@ -2,7 +2,7 @@ import type { Cookie } from '../cookie/cookie'
 import { CookieJar } from '../cookie/cookieJar'
 import { MemoryCookieStore } from '../memstore'
 import { Store } from '../store'
-import type { Callback } from '../utils'
+import type { Callback, ErrorCallback } from '../utils'
 
 const url = 'http://example.com/index.html'
 
@@ -33,24 +33,20 @@ describe('store removeAllCookies API', () => {
       // replace remove cookie behavior to throw an error on the 4th invocation
       const _removeCookie = store.removeCookie.bind(store)
       const spy = jest.spyOn(store, 'removeCookie')
-      spy.mockImplementationOnce(
-        (domain: string, path: string, key: string, callback: Callback<void>) =>
-          _removeCookie.call(store, domain, path, key, callback),
+      spy.mockImplementationOnce((domain, path, key, callback) =>
+        _removeCookie.call(store, domain, path, key, callback),
       )
-      spy.mockImplementationOnce(
-        (domain: string, path: string, key: string, callback: Callback<void>) =>
-          _removeCookie.call(store, domain, path, key, callback),
+      spy.mockImplementationOnce((domain, path, key, callback) =>
+        _removeCookie.call(store, domain, path, key, callback),
       )
-      spy.mockImplementationOnce(
-        (domain: string, path: string, key: string, callback: Callback<void>) =>
-          _removeCookie.call(store, domain, path, key, callback),
+      spy.mockImplementationOnce((domain, path, key, callback) =>
+        _removeCookie.call(store, domain, path, key, callback),
       )
-      spy.mockImplementationOnce(
-        (_domain, _path, _key, callback: Callback<void>) =>
-          callback(new Error('something happened 4')),
+      spy.mockImplementationOnce((_domain, _path, _key, callback) =>
+        callback(new Error('something happened 4')),
       )
 
-      await expect(jar.removeAllCookies()).rejects.toThrowError(
+      await expect(jar.removeAllCookies()).rejects.toThrow(
         'something happened 4',
       )
 
@@ -73,21 +69,14 @@ describe('store removeAllCookies API', () => {
       // replace remove cookie behavior to throw an error on the 4th invocation
       const _removeCookie = store.removeCookie.bind(store)
       const spy = jest.spyOn(store, 'removeCookie')
-      spy.mockImplementation(
-        (
-          domain: string,
-          path: string,
-          key: string,
-          callback: Callback<void>,
-        ) => {
-          if (spy.mock.calls.length % 2 === 1) {
-            return callback(
-              new Error(`something happened ${spy.mock.calls.length}`),
-            )
-          }
-          return _removeCookie.call(store, domain, path, key, callback)
-        },
-      )
+      spy.mockImplementation((domain, path, key, callback) => {
+        if (spy.mock.calls.length % 2 === 1) {
+          return callback(
+            new Error(`something happened ${spy.mock.calls.length}`),
+          )
+        }
+        return _removeCookie.call(store, domain, path, key, callback)
+      })
 
       await expect(jar.removeAllCookies()).rejects.toThrowError(
         'something happened 1',
@@ -181,8 +170,8 @@ class StoreWithoutRemoveAll extends Store {
   }
 
   override putCookie(cookie: Cookie): Promise<void>
-  override putCookie(cookie: Cookie, callback: Callback<void>): void
-  override putCookie(cookie: Cookie, callback?: Callback<void>): unknown {
+  override putCookie(cookie: Cookie, callback: ErrorCallback): void
+  override putCookie(cookie: Cookie, callback?: ErrorCallback): unknown {
     this.stats.put++
     this.cookies.push(cookie)
     if (!callback) {
@@ -210,13 +199,13 @@ class StoreWithoutRemoveAll extends Store {
     domain: string,
     path: string,
     key: string,
-    callback: Callback<void>,
+    callback: ErrorCallback,
   ): void
   override removeCookie(
     _domain: string,
     _path: string,
     _key: string,
-    callback?: Callback<void>,
+    callback?: ErrorCallback,
   ): unknown {
     this.stats.remove++
     if (!callback) {
@@ -257,13 +246,13 @@ class MemoryStoreExtension extends MemoryCookieStore {
     domain: string,
     path: string,
     key: string,
-    callback: Callback<void>,
+    callback: ErrorCallback,
   ): void
   override removeCookie(
     domain: string,
     path: string,
     key: string,
-    callback?: Callback<void>,
+    callback?: ErrorCallback,
   ): unknown {
     this.stats.remove++
     if (!callback) {
@@ -273,8 +262,8 @@ class MemoryStoreExtension extends MemoryCookieStore {
   }
 
   override removeAllCookies(): Promise<void>
-  override removeAllCookies(callback: Callback<void>): void
-  override removeAllCookies(callback?: Callback<void>): unknown {
+  override removeAllCookies(callback: ErrorCallback): void
+  override removeAllCookies(callback?: ErrorCallback): unknown {
     this.stats.removeAll++
     if (!callback) {
       throw new Error('This should not be undefined')

lib/cookie/cookieJar.ts

@@ -8,6 +8,7 @@ import { pathMatch } from '../pathMatch'
 import { Cookie } from './cookie'
 import {
   Callback,
+  ErrorCallback,
   createPromiseCallback,
   inOperator,
   safeToString,
@@ -169,15 +170,19 @@ export class CookieJar {
     this.store = store ?? new MemoryCookieStore()
   }
 
-  private callSync<T>(fn: (callback: Callback<T>) => void): T | undefined {
+  private callSync<T>(
+    // Using tuples is needed to check if `T` is `never` because `T extends never ? true : false`
+    // evaluates to `never` instead of `true`.
+    fn: (callback: [T] extends [never] ? ErrorCallback : Callback<T>) => void,
+  ): T | undefined {
     if (!this.store.synchronous) {
       throw new Error(
         'CookieJar store is not synchronous; use async API instead.',
       )
     }
     let syncErr: Error | null = null
     let syncResult: T | undefined = undefined
-    fn.call(this, (error, result) => {
+    fn.call(this, (error: Error | null, result?: T | undefined) => {
       syncErr = error
       syncResult = result
     })
@@ -413,22 +418,14 @@ export class CookieJar {
     const store = this.store
 
     if (!store.updateCookie) {
-      store.updateCookie = function (
+      store.updateCookie = async function (
         _oldCookie: Cookie,
         newCookie: Cookie,
-        cb?: Callback<void>,
+        cb?: ErrorCallback,
       ): Promise<void> {
         return this.putCookie(newCookie).then(
-          () => {
-            if (cb) {
-              cb(null, undefined)
-            }
-          },
-          (error: Error) => {
-            if (cb) {
-              cb(error, undefined)
-            }
-          },
+          () => cb?.(null),
+          (error: Error) => cb?.(error),
         )
       }
     }
@@ -492,13 +489,10 @@ export class CookieJar {
     url: string,
     options?: SetCookieOptions,
   ): Cookie | undefined {
-    const setCookieFn = this.setCookie.bind(
-      this,
-      cookie,
-      url,
-      options as SetCookieOptions,
-    )
-    return this.callSync<Cookie | undefined>(setCookieFn)
+    const setCookieFn = options
+      ? this.setCookie.bind(this, cookie, url, options)
+      : this.setCookie.bind(this, cookie, url)
+    return this.callSync(setCookieFn)
   }
 
   // RFC6365 S5.4
@@ -659,9 +653,7 @@ export class CookieJar {
     return promiseCallback.promise
   }
   getCookiesSync(url: string, options?: GetCookiesOptions): Cookie[] {
-    return (
-      this.callSync<Cookie[]>(this.getCookies.bind(this, url, options)) ?? []
-    )
+    return this.callSync(this.getCookies.bind(this, url, options)) ?? []
   }
 
   getCookieString(
@@ -686,19 +678,14 @@ export class CookieJar {
       options = undefined
     }
     const promiseCallback = createPromiseCallback(callback)
-    const next: Callback<Cookie[]> = function (
-      err: Error | null,
-      cookies: Cookie[] | undefined,
-    ) {
+    const next: Callback<Cookie[]> = function (err, cookies) {
       if (err) {
         promiseCallback.callback(err)
-      } else if (cookies === undefined) {
-        promiseCallback.callback(null, cookies)
       } else {
         promiseCallback.callback(
           null,
           cookies
-            .sort(cookieCompare)
+            ?.sort(cookieCompare)
             .map((c) => c.cookieString())
             .join('; '),
         )
@@ -710,8 +697,10 @@ export class CookieJar {
   }
   getCookieStringSync(url: string, options?: GetCookiesOptions): string {
     return (
-      this.callSync<string | undefined>(
-        this.getCookieString.bind(this, url, options as GetCookiesOptions),
+      this.callSync(
+        options
+          ? this.getCookieString.bind(this, url, options)
+          : this.getCookieString.bind(this, url),
       ) ?? ''
     )
   }
@@ -773,9 +762,7 @@ export class CookieJar {
     options: GetCookiesOptions = {},
   ): string[] {
     return (
-      this.callSync<string[] | undefined>(
-        this.getSetCookieStrings.bind(this, url, options),
-      ) ?? []
+      this.callSync(this.getSetCookieStrings.bind(this, url, options)) ?? []
     )
   }
 
@@ -851,9 +838,7 @@ export class CookieJar {
     return promiseCallback.promise
   }
   serializeSync(): SerializedCookieJar | undefined {
-    return this.callSync<SerializedCookieJar>((callback) => {
-      this.serialize(callback)
-    })
+    return this.callSync((callback) => this.serialize(callback))
   }
 
   toJSON() {
@@ -959,10 +944,10 @@ export class CookieJar {
     return this._cloneSync(newStore)
   }
 
-  removeAllCookies(callback: Callback<void>): void
+  removeAllCookies(callback: ErrorCallback): void
   removeAllCookies(): Promise<void>
-  removeAllCookies(callback?: Callback<void>): unknown {
-    const promiseCallback = createPromiseCallback<void>(callback)
+  removeAllCookies(callback?: ErrorCallback): unknown {
+    const promiseCallback = createPromiseCallback<undefined>(callback)
     const cb = promiseCallback.callback
 
     const store = this.store
@@ -974,7 +959,9 @@ export class CookieJar {
       typeof store.removeAllCookies === 'function' &&
       store.removeAllCookies !== Store.prototype.removeAllCookies
     ) {
-      void store.removeAllCookies(cb)
+      // `Callback<undefined>` and `ErrorCallback` are *technically* incompatible, but for the
+      // standard implementation `cb = (err, result) => {}`, they're essentially the same.
+      void store.removeAllCookies(cb as ErrorCallback)
       return promiseCallback.promise
     }
 
@@ -989,7 +976,7 @@ export class CookieJar {
       }
 
       if (cookies.length === 0) {
-        cb(null)
+        cb(null, undefined)
         return
       }
 
@@ -1005,7 +992,7 @@ export class CookieJar {
 
         if (completedCount === cookies?.length) {
           if (removeErrors[0]) cb(removeErrors[0])
-          else cb(null)
+          else cb(null, undefined)
           return
         }
       }
@@ -1023,7 +1010,9 @@ export class CookieJar {
     return promiseCallback.promise
   }
   removeAllCookiesSync(): void {
-    return this.callSync<void>((callback) => this.removeAllCookies(callback))
+    return this.callSync<never>((callback) => {
+      return this.removeAllCookies(callback)
+    })
   }
 
   static deserialize(