diff --git a/.eslintrc.json b/.eslintrc.json index 06fcc019..52793e7c 100644 --- a/.eslintrc.json +++ b/.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] } } diff --git a/lib/__tests__/removeAll.spec.ts b/lib/__tests__/removeAll.spec.ts index 3c0db89a..08050419 100644 --- a/lib/__tests__/removeAll.spec.ts +++ b/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) => - _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) => - _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) => - _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) => - 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, - ) => { - 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 - override putCookie(cookie: Cookie, callback: Callback): void - override putCookie(cookie: Cookie, callback?: Callback): 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, + callback: ErrorCallback, ): void override removeCookie( _domain: string, _path: string, _key: string, - callback?: Callback, + callback?: ErrorCallback, ): unknown { this.stats.remove++ if (!callback) { @@ -257,13 +246,13 @@ class MemoryStoreExtension extends MemoryCookieStore { domain: string, path: string, key: string, - callback: Callback, + callback: ErrorCallback, ): void override removeCookie( domain: string, path: string, key: string, - callback?: Callback, + callback?: ErrorCallback, ): unknown { this.stats.remove++ if (!callback) { @@ -273,8 +262,8 @@ class MemoryStoreExtension extends MemoryCookieStore { } override removeAllCookies(): Promise - override removeAllCookies(callback: Callback): void - override removeAllCookies(callback?: Callback): unknown { + override removeAllCookies(callback: ErrorCallback): void + override removeAllCookies(callback?: ErrorCallback): unknown { this.stats.removeAll++ if (!callback) { throw new Error('This should not be undefined') diff --git a/lib/cookie/cookieJar.ts b/lib/cookie/cookieJar.ts index b20511cd..32cdd9b7 100644 --- a/lib/cookie/cookieJar.ts +++ b/lib/cookie/cookieJar.ts @@ -8,6 +8,7 @@ import { pathMatch } from '../pathMatch' import { Cookie } from './cookie' import { Callback, + ErrorCallback, createPromiseCallback, inOperator, safeToString, @@ -169,7 +170,11 @@ export class CookieJar { this.store = store ?? new MemoryCookieStore() } - private callSync(fn: (callback: Callback) => void): T | undefined { + private callSync( + // 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) => void, + ): T | undefined { if (!this.store.synchronous) { throw new Error( 'CookieJar store is not synchronous; use async API instead.', @@ -177,7 +182,7 @@ export class CookieJar { } 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, + cb?: ErrorCallback, ): Promise { 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(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(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 = function ( - err: Error | null, - cookies: Cookie[] | undefined, - ) { + const next: Callback = 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( - 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( - 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((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 + removeAllCookies(callback: ErrorCallback): void removeAllCookies(): Promise - removeAllCookies(callback?: Callback): unknown { - const promiseCallback = createPromiseCallback(callback) + removeAllCookies(callback?: ErrorCallback): unknown { + const promiseCallback = createPromiseCallback(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` 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((callback) => this.removeAllCookies(callback)) + return this.callSync((callback) => { + return this.removeAllCookies(callback) + }) } static deserialize( diff --git a/lib/memstore.ts b/lib/memstore.ts index 31f8ea26..c14be565 100644 --- a/lib/memstore.ts +++ b/lib/memstore.ts @@ -34,7 +34,12 @@ import { pathMatch } from './pathMatch' import { permuteDomain } from './permuteDomain' import { Store } from './store' import { getCustomInspectSymbol, getUtilInspect } from './utilHelper' -import { type Callback, createPromiseCallback, inOperator } from './utils' +import { + Callback, + createPromiseCallback, + inOperator, + ErrorCallback, +} from './utils' export type MemoryCookieStoreIndex = { [domain: string]: { @@ -119,7 +124,6 @@ export class MemoryCookieStore extends Store { const results: Cookie[] = [] const promiseCallback = createPromiseCallback(callback) - const cb = promiseCallback.callback if (!domain) { return promiseCallback.resolve([]) @@ -169,20 +173,17 @@ export class MemoryCookieStore extends Store { pathMatcher(domainIndex) }) - cb(null, results) - return promiseCallback.promise + return promiseCallback.resolve(results) } override putCookie(cookie: Cookie): Promise - override putCookie(cookie: Cookie, callback: Callback): void - override putCookie(cookie: Cookie, callback?: Callback): unknown { - const promiseCallback = createPromiseCallback(callback) - const cb = promiseCallback.callback + override putCookie(cookie: Cookie, callback: ErrorCallback): void + override putCookie(cookie: Cookie, callback?: ErrorCallback): unknown { + const promiseCallback = createPromiseCallback(callback) const { domain, path, key } = cookie if (domain == null || path == null || key == null) { - cb(null, undefined) - return promiseCallback.promise + return promiseCallback.resolve(undefined) } const domainEntry = @@ -199,29 +200,26 @@ export class MemoryCookieStore extends Store { pathEntry[key] = cookie - cb(null, undefined) - - return promiseCallback.promise + return promiseCallback.resolve(undefined) } override updateCookie(oldCookie: Cookie, newCookie: Cookie): Promise override updateCookie( oldCookie: Cookie, newCookie: Cookie, - callback: Callback, + callback: ErrorCallback, ): void override updateCookie( _oldCookie: Cookie, newCookie: Cookie, - callback?: Callback, + callback?: ErrorCallback, ): unknown { - // this seems wrong but it stops typescript from complaining and all the test pass... - callback = callback ?? function () {} - // updateCookie() may avoid updating cookies that are identical. For example, // lastAccessed may not be important to some stores and an equality // comparison could exclude that field. - return this.putCookie(newCookie, callback) + return callback + ? this.putCookie(newCookie, callback) + : this.putCookie(newCookie) } override removeCookie( @@ -233,34 +231,31 @@ export class MemoryCookieStore extends Store { domain: string, path: string, key: string, - callback: Callback, + callback: ErrorCallback, ): void override removeCookie( domain: string, path: string, key: string, - callback?: Callback, + callback?: ErrorCallback, ): unknown { - const promiseCallback = createPromiseCallback(callback) - const cb = promiseCallback.callback + const promiseCallback = createPromiseCallback(callback) delete this.idx[domain]?.[path]?.[key] - cb(null, undefined) - return promiseCallback.promise + return promiseCallback.resolve(undefined) } override removeCookies(domain: string, path: string): Promise override removeCookies( domain: string, path: string, - callback: Callback, + callback: ErrorCallback, ): void override removeCookies( domain: string, path: string, - callback?: Callback, + callback?: ErrorCallback, ): unknown { - const promiseCallback = createPromiseCallback(callback) - const cb = promiseCallback.callback + const promiseCallback = createPromiseCallback(callback) const domainEntry = this.idx[domain] if (domainEntry) { @@ -271,27 +266,21 @@ export class MemoryCookieStore extends Store { } } - cb(null) - return promiseCallback.promise + return promiseCallback.resolve(undefined) } override removeAllCookies(): Promise - override removeAllCookies(callback: Callback): void - override removeAllCookies(callback?: Callback): unknown { - const promiseCallback = createPromiseCallback(callback) - const cb = promiseCallback.callback - + override removeAllCookies(callback: ErrorCallback): void + override removeAllCookies(callback?: ErrorCallback): unknown { + const promiseCallback = createPromiseCallback(callback) this.idx = Object.create(null) as MemoryCookieStoreIndex - - cb(null) - return promiseCallback.promise + return promiseCallback.resolve(undefined) } override getAllCookies(): Promise override getAllCookies(callback: Callback): void override getAllCookies(callback?: Callback): unknown { const promiseCallback = createPromiseCallback(callback) - const cb = promiseCallback.callback const cookies: Cookie[] = [] const idx = this.idx @@ -318,8 +307,7 @@ export class MemoryCookieStore extends Store { return (a.creationIndex || 0) - (b.creationIndex || 0) }) - cb(null, cookies) - return promiseCallback.promise + return promiseCallback.resolve(cookies) } } diff --git a/lib/store.ts b/lib/store.ts index ed5d6b73..545c4723 100644 --- a/lib/store.ts +++ b/lib/store.ts @@ -36,7 +36,7 @@ 'use strict' import type { Cookie } from './cookie/cookie' -import type { Callback } from './utils' +import type { Callback, ErrorCallback } from './utils' export class Store { synchronous: boolean @@ -86,8 +86,8 @@ export class Store { } putCookie(cookie: Cookie): Promise - putCookie(cookie: Cookie, callback: Callback): void - putCookie(_cookie: Cookie, _callback?: Callback): unknown { + putCookie(cookie: Cookie, callback: ErrorCallback): void + putCookie(_cookie: Cookie, _callback?: ErrorCallback): unknown { throw new Error('putCookie is not implemented') } @@ -95,12 +95,12 @@ export class Store { updateCookie( oldCookie: Cookie, newCookie: Cookie, - callback: Callback, + callback: ErrorCallback, ): void updateCookie( _oldCookie: Cookie, _newCookie: Cookie, - _callback?: Callback, + _callback?: ErrorCallback, ): unknown { // recommended default implementation: // return this.putCookie(newCookie, cb); @@ -116,13 +116,13 @@ export class Store { domain: string | null | undefined, path: string | null | undefined, key: string | null | undefined, - callback: Callback, + callback: ErrorCallback, ): void removeCookie( _domain: string | null | undefined, _path: string | null | undefined, _key: string | null | undefined, - _callback?: Callback, + _callback?: ErrorCallback, ): unknown { throw new Error('removeCookie is not implemented') } @@ -131,19 +131,19 @@ export class Store { removeCookies( domain: string, path: string | null, - callback: Callback, + callback: ErrorCallback, ): void removeCookies( _domain: string, _path: string | null, - _callback?: Callback, + _callback?: ErrorCallback, ): unknown { throw new Error('removeCookies is not implemented') } removeAllCookies(): Promise - removeAllCookies(callback: Callback): void - removeAllCookies(_callback?: Callback): unknown { + removeAllCookies(callback: ErrorCallback): void + removeAllCookies(_callback?: ErrorCallback): unknown { throw new Error('removeAllCookies is not implemented') } diff --git a/lib/utils.ts b/lib/utils.ts index effb02b2..0a5bb07d 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -4,6 +4,11 @@ export interface Callback { (error: null, result: T): void } +/** A callback function that only accepts an error. */ +export interface ErrorCallback { + (error: Error | null): void +} + /** Wrapped `Object.prototype.toString`, so that you don't need to remember to use `.call()`. */ export const objectToString = (obj: unknown) => Object.prototype.toString.call(obj)