clean up usage of null/undefined

lib/cookie/canonicalDomain.ts

@@ -1,8 +1,9 @@
-import * as punycode from 'punycode/punycode.js'
+import { toASCII } from 'punycode/punycode.js'
 import { IP_V6_REGEX_OBJECT } from './constants'
+import type { Nullable } from '../utils'
 
 // S5.1.2 Canonicalized Host Names
-export function canonicalDomain(str: string | null) {
+export function canonicalDomain(str: Nullable<string>): string | null {
   if (str == null) {
     return null
   }
@@ -15,7 +16,7 @@ export function canonicalDomain(str: string | null) {
   // convert to IDN if any non-ASCII characters
   // eslint-disable-next-line no-control-regex
   if (/[^\u0001-\u007f]/.test(_str)) {
-    _str = punycode.toASCII(_str)
+    _str = toASCII(_str)
   }
 
   return _str.toLowerCase()

lib/cookie/cookie.ts

@@ -112,9 +112,19 @@ type ParseCookieOptions = {
   loose?: boolean | undefined
 }
 
+/**
+ * Parses a string into a Cookie object.
+ * @param str Cookie string to parse
+ * @returns `Cookie` object for valid string inputs, `undefined` for invalid string inputs,
+ * or `null` for non-string inputs or empty string
+ */
 function parse(
   str: string,
   options?: ParseCookieOptions,
+  // TBD: Should we change the API to have a single "invalid input" return type? I think `undefined`
+  // would be more consistent with the rest of the code, and it would be of minimal impact. Only
+  // users who are passing an invalid input and doing an explicit null check would be broken, and
+  // that doesn't seem like it would be a significant number of users.
 ): Cookie | undefined | null {
   if (validators.isEmptyString(str) || !validators.isString(str)) {
     return null
@@ -365,6 +375,17 @@ function fromJSON(str: unknown) {
   return c
 }
 
+type CreateCookieOptions = Omit<
+  {
+    // Assume that all non-method attributes on the class can be configured, except creationIndex.
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    [K in keyof Cookie as Cookie[K] extends (...args: any[]) => any
+      ? never
+      : K]?: Cookie[K]
+  },
+  'creationIndex'
+>
+
 const cookieDefaults = {
   // the order in which the RFC has them:
   key: '',
@@ -382,46 +403,42 @@ const cookieDefaults = {
   creation: null,
   lastAccessed: null,
   sameSite: undefined,
-}
-
-type CreateCookieOptions = {
-  key?: string
-  value?: string
-  expires?: Date | 'Infinity' | null
-  maxAge?: number | 'Infinity' | '-Infinity'
-  domain?: string | null
-  path?: string | null
-  secure?: boolean
-  httpOnly?: boolean
-  extensions?: string[] | null
-  creation?: Date | 'Infinity' | null
-  creationIndex?: number
-  hostOnly?: boolean | null
-  pathIsDefault?: boolean | null
-  lastAccessed?: Date | 'Infinity' | null
-  sameSite?: string | undefined
-}
+} as const satisfies Required<CreateCookieOptions>
 
 export class Cookie {
-  key: string | undefined
-  value: string | undefined
-  expires: Date | 'Infinity' | null | undefined
-  maxAge: number | 'Infinity' | '-Infinity' | undefined
-  domain: string | null | undefined
-  path: string | null | undefined
-  secure: boolean | undefined
-  httpOnly: boolean | undefined
-  extensions: string[] | null | undefined
+  key: string
+  value: string
+  expires: Date | 'Infinity' | null
+  maxAge: number | 'Infinity' | '-Infinity' | null
+  domain: string | null
+  path: string | null
+  secure: boolean
+  httpOnly: boolean
+  extensions: string[] | null
   creation: Date | 'Infinity' | null
-  creationIndex: number | undefined
-  hostOnly: boolean | null | undefined
-  pathIsDefault: boolean | null | undefined
-  lastAccessed: Date | 'Infinity' | null | undefined
+  creationIndex: number
+  hostOnly: boolean | null
+  pathIsDefault: boolean | null
+  lastAccessed: Date | 'Infinity' | null
   sameSite: string | undefined
 
   constructor(options: CreateCookieOptions = {}) {
-    Object.assign(this, cookieDefaults, options)
-    this.creation = options.creation ?? cookieDefaults.creation ?? new Date()
+    this.key = options.key ?? cookieDefaults.key
+    this.value = options.value ?? cookieDefaults.value
+    this.expires = options.expires ?? cookieDefaults.expires
+    this.maxAge = options.maxAge ?? cookieDefaults.maxAge
+    this.domain = options.domain ?? cookieDefaults.domain
+    this.path = options.path ?? cookieDefaults.path
+    this.secure = options.secure ?? cookieDefaults.secure
+    this.httpOnly = options.httpOnly ?? cookieDefaults.httpOnly
+    this.extensions = options.extensions ?? cookieDefaults.extensions
+    this.creation = options.creation ?? cookieDefaults.creation
+    this.hostOnly = options.hostOnly ?? cookieDefaults.hostOnly
+    this.pathIsDefault = options.pathIsDefault ?? cookieDefaults.pathIsDefault
+    this.lastAccessed = options.lastAccessed ?? cookieDefaults.lastAccessed
+    this.sameSite = options.sameSite ?? cookieDefaults.sameSite
+
+    this.creation = options.creation ?? new Date()
 
     // used to break creation ties in cookieCompare():
     Object.defineProperty(this, 'creationIndex', {
@@ -430,6 +447,8 @@ export class Cookie {
       writable: true,
       value: ++Cookie.cookiesCreated,
     })
+    // Duplicate operation, but it makes TypeScript happy...
+    this.creationIndex = Cookie.cookiesCreated
   }
 
   [Symbol.for('nodejs.util.inspect.custom')]() {

lib/cookie/cookieJar.ts

@@ -9,8 +9,9 @@ import { pathMatch } from '../pathMatch'
 import { Cookie } from './cookie'
 import {
   Callback,
-  createPromiseCallback,
   ErrorCallback,
+  Nullable,
+  createPromiseCallback,
   inOperator,
   safeToString,
 } from '../utils'
@@ -154,7 +155,7 @@ export class CookieJar {
   readonly prefixSecurity: string
 
   constructor(
-    store?: Store | null | undefined,
+    store?: Nullable<Store>,
     options?: CreateCookieJarOptions | boolean,
   ) {
     if (typeof options === 'boolean') {
@@ -433,16 +434,16 @@ export class CookieJar {
       }
     }
 
-    function withCookie(
-      err: Error | null,
-      oldCookie: Cookie | undefined | null,
+    const withCookie: Callback<Nullable<Cookie>> = function withCookie(
+      err,
+      oldCookie,
     ): void {
       if (err) {
         cb(err)
         return
       }
 
-      const next = function (err: Error | null): void {
+      const next: ErrorCallback = function (err) {
         if (err) {
           cb(err)
         } else if (typeof cookie === 'string') {
@@ -484,6 +485,7 @@ export class CookieJar {
       }
     }
 
+    // TODO: Refactor to avoid using a callback
     store.findCookie(cookie.domain, cookie.path, cookie.key, withCookie)
     return promiseCallback.promise
   }
@@ -741,18 +743,13 @@ export class CookieJar {
       callback,
     )
 
-    const next: Callback<Cookie[]> = function (
-      err: Error | null,
-      cookies: Cookie[] | undefined,
-    ) {
+    const next: Callback<Cookie[] | undefined> = function (err, cookies) {
       if (err) {
         promiseCallback.callback(err)
-      } else if (cookies === undefined) {
-        promiseCallback.callback(null, undefined)
       } else {
         promiseCallback.callback(
           null,
-          cookies.map((c) => {
+          cookies?.map((c) => {
             return c.toString()
           }),
         )
@@ -872,7 +869,7 @@ export class CookieJar {
 
     cookies = cookies.slice() // do not modify the original
 
-    const putNext = (err: Error | null): void => {
+    const putNext: ErrorCallback = (err) => {
       if (err) {
         return callback(err, undefined)
       }
@@ -988,7 +985,8 @@ export class CookieJar {
       let completedCount = 0
       const removeErrors: Error[] = []
 
-      function removeCookieCb(removeErr: Error | null) {
+      // TODO: Refactor to avoid using callback
+      const removeCookieCb: ErrorCallback = function removeCookieCb(removeErr) {
         if (removeErr) {
           removeErrors.push(removeErr)
         }

lib/cookie/defaultPath.ts

@@ -1,12 +1,14 @@
 // RFC6265 S5.1.4 Paths and Path-Match
 
+import type { Nullable } from '../utils'
+
 /*
  * "The user agent MUST use an algorithm equivalent to the following algorithm
  * to compute the default-path of a cookie:"
  *
  * Assumption: the path (and not query part or absolute uri) is passed in.
  */
-export function defaultPath(path?: string | null): string {
+export function defaultPath(path?: Nullable<string>): string {
   // "2. If the uri-path is empty or if the first character of the uri-path is not
   // a %x2F ("/") character, output %x2F ("/") and skip the remaining steps.
   if (!path || path.slice(0, 1) !== '/') {

lib/cookie/domainMatch.ts

@@ -1,3 +1,4 @@
+import type { Nullable } from '../utils'
 import { canonicalDomain } from './canonicalDomain'
 
 // Dumped from [email protected], with the following changes:
@@ -9,16 +10,16 @@ const IP_REGEX_LOWERCASE =
 
 // S5.1.3 Domain Matching
 export function domainMatch(
-  str?: string | null,
-  domStr?: string | null,
+  str?: Nullable<string>,
+  domStr?: Nullable<string>,
   canonicalize?: boolean,
 ): boolean | null {
   if (str == null || domStr == null) {
     return null
   }
 
-  let _str: string | null
-  let _domStr: string | null
+  let _str: Nullable<string>
+  let _domStr: Nullable<string>
 
   if (canonicalize !== false) {
     _str = canonicalDomain(str)

lib/cookie/parseDate.ts

@@ -1,4 +1,7 @@
 // date-time parsing constants (RFC6265 S5.1.1)
+
+import type { Nullable } from '../utils'
+
 // eslint-disable-next-line no-control-regex
 const DATE_DELIM = /[\x09\x20-\x2F\x3B-\x40\x5B-\x60\x7B-\x7E]/
 
@@ -123,7 +126,7 @@ function parseMonth(token: string) {
 /*
  * RFC6265 S5.1.1 date parser (see RFC for full grammar)
  */
-export function parseDate(str: string | undefined | null): Date | undefined {
+export function parseDate(str: Nullable<string>): Date | undefined {
   if (!str) {
     return undefined
   }

lib/memstore.ts

@@ -33,7 +33,12 @@ import type { Cookie } from './cookie/cookie'
 import { pathMatch } from './pathMatch'
 import { permuteDomain } from './permuteDomain'
 import { Store } from './store'
-import { Callback, createPromiseCallback, ErrorCallback } from './utils'
+import {
+  Callback,
+  createPromiseCallback,
+  ErrorCallback,
+  Nullable,
+} from './utils'
 
 export type MemoryCookieStoreIndex = {
   [domain: string]: {
@@ -54,20 +59,20 @@ export class MemoryCookieStore extends Store {
   }
 
   override findCookie(
-    domain: string | null,
-    path: string | null,
-    key: string | undefined,
-  ): Promise<Cookie | undefined>
+    domain: Nullable<string>,
+    path: Nullable<string>,
+    key: Nullable<string>,
+  ): Promise<Nullable<Cookie>>
   override findCookie(
-    domain: string | null,
-    path: string | null,
-    key: string | undefined,
+    domain: Nullable<string>,
+    path: Nullable<string>,
+    key: Nullable<string>,
     callback: Callback<Cookie | undefined>,
   ): void
   override findCookie(
-    domain: string | null,
-    path: string | null,
-    key: string | undefined,
+    domain: Nullable<string>,
+    path: Nullable<string>,
+    key: Nullable<string>,
     callback?: Callback<Cookie | undefined>,
   ): unknown {
     const promiseCallback = createPromiseCallback(callback)