From a5dd9aa37e7891b2620798560799c7f8bd380877 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 30 Jul 2021 17:58:59 +0200 Subject: [PATCH] General code improvements --- source/core/index.ts | 6 +-- source/core/options.ts | 80 ++++++++----------------------- source/core/utils/proxy-events.ts | 2 +- source/create.ts | 8 ++-- source/types.ts | 4 +- 5 files changed, 28 insertions(+), 72 deletions(-) diff --git a/source/core/index.ts b/source/core/index.ts index 262d05b5d..5287473f1 100644 --- a/source/core/index.ts +++ b/source/core/index.ts @@ -32,14 +32,14 @@ import { import type {PlainResponse} from './response.js'; import type {PromiseCookieJar, NativeRequestOptions, RetryOptions} from './options.js'; +type Error = NodeJS.ErrnoException; + export interface Progress { percent: number; transferred: number; total?: number; } -type Error = NodeJS.ErrnoException; - const supportsBrotli = is.string(process.versions.brotli); const methodsWithoutBody: ReadonlySet = new Set(['GET', 'HEAD']); @@ -125,7 +125,7 @@ const proxiedRequestEvents = [ 'continue', 'information', 'upgrade', -]; +] as const; const noop = () => {}; diff --git a/source/core/options.ts b/source/core/options.ts index ffacce916..b10b9f909 100644 --- a/source/core/options.ts +++ b/source/core/options.ts @@ -299,8 +299,8 @@ export interface RetryOptions { maxRetryAfter?: number; } -export type CreateConnectionFunction = (options: NativeRequestOptions, oncreate: (error: Error, socket: Socket) => void) => Socket; -export type CheckServerIdentityFunction = (hostname: string, certificate: DetailedPeerCertificate) => Error | void; +export type CreateConnectionFunction = (options: NativeRequestOptions, oncreate: (error: NodeJS.ErrnoException, socket: Socket) => void) => Socket; +export type CheckServerIdentityFunction = (hostname: string, certificate: DetailedPeerCertificate) => NodeJS.ErrnoException | void; export interface CacheOptions { shared?: boolean; @@ -768,7 +768,7 @@ export default class Options { private _merging: boolean; private readonly _init: OptionsInit[]; - constructor(input?: string | URL | OptionsInit, options?: OptionsInit, defaults?: Options | OptionsInit) { + constructor(input?: string | URL | OptionsInit, options?: OptionsInit, defaults?: Options) { assert.any([is.string, is.urlInstance, is.object, is.undefined], input); assert.any([is.object, is.undefined], options); assert.any([is.object, is.undefined], defaults); @@ -777,8 +777,8 @@ export default class Options { throw new TypeError('The defaults must be passed as the third argument'); } - this._internals = cloneInternals((defaults as Options)?._internals ?? defaults ?? defaultInternals); - this._init = [...((defaults as Options)?._init ?? [])]; + this._internals = cloneInternals(defaults?._internals ?? defaults ?? defaultInternals); + this._init = [...(defaults?._init ?? [])]; this._merging = false; this._unixOptions = undefined; @@ -1495,24 +1495,30 @@ export default class Options { throw new Error(`Unexpected hook event: ${knownHookEvent}`); } - const hooks: unknown[] = value[knownHookEvent as keyof Hooks]; + const typedKnownHookEvent = knownHookEvent as keyof Hooks; + const typedValue = value as Hooks; + const hooks = typedValue[typedKnownHookEvent]; + assert.any([is.array, is.undefined], hooks); - for (const hook of hooks) { - assert.function_(hook); + if (hooks) { + for (const hook of hooks) { + assert.function_(hook); + } } if (this._merging) { if (hooks) { // @ts-expect-error FIXME - this._internals.hooks[knownHookEvent].push(...hooks); + this._internals.hooks[typedKnownHookEvent].push(...hooks); } - } else if (hooks) { - // @ts-expect-error FIXME - this._internals.hooks[knownHookEvent] = [...hooks]; } else { + if (!hooks) { + throw new Error(`Missing hook event: ${knownHookEvent}`); + } + // @ts-expect-error FIXME - this._internals.hooks[knownHookEvent] = []; + this._internals.hooks[knownHookEvent] = [...hooks]; } } } @@ -2257,51 +2263,3 @@ export default class Options { Object.freeze(options.context); } } - -// It's user responsibility to make sensitive data in `context` non-enumerable -const nonEnumerableProperties = new Set([ - // Functions - 'constructor', - 'merge', - 'tryMerge', - 'createNativeRequestOptions', - 'getRequestFunction', - 'getFallbackRequestFunction', - 'freeze', - - // Payload - 'body', - 'form', - 'json', - - // Getters that always throw - 'auth', - 'followRedirects', - 'searchParameters', - - // May contain sensitive data - 'username', - 'password', - 'headers', - 'searchParams', - 'url', - - // Privates - '_unixOptions', - '_internals', - '_merging', - '_init', -]); - -// We want all the properties to be enumerable, so people instead doing -// `util.inspect(options, {getters: true, showHidden: true})` -// can do just `util.inspect(options, {getters: true})`. -const propertyDescriptors: PropertyDescriptorMap = {}; -const keys = Object.getOwnPropertyNames(Options.prototype).filter(property => !nonEnumerableProperties.has(property)); -const makeEnumerable = {enumerable: true}; - -for (const key of keys) { - propertyDescriptors[key] = makeEnumerable; -} - -Object.defineProperties(Options.prototype, propertyDescriptors); diff --git a/source/core/utils/proxy-events.ts b/source/core/utils/proxy-events.ts index ee562b4b2..2e9fe7fdd 100644 --- a/source/core/utils/proxy-events.ts +++ b/source/core/utils/proxy-events.ts @@ -3,7 +3,7 @@ import {EventEmitter} from 'events'; type Fn = (...args: unknown[]) => void; type Fns = Record; -export default function proxyEvents(from: EventEmitter, to: EventEmitter, events: string[]): () => void { +export default function proxyEvents(from: EventEmitter, to: EventEmitter, events: Readonly): () => void { const eventFunctions: Fns = {}; for (const event of events) { diff --git a/source/create.ts b/source/create.ts index 9ebc2b6be..fadb50bfb 100644 --- a/source/create.ts +++ b/source/create.ts @@ -23,9 +23,7 @@ const delay = async (ms: number) => new Promise(resolve => { setTimeout(resolve, ms); }); -const isGotInstance = (value: Got | ExtendOptions): value is Got => ( - 'defaults' in value && 'options' in value.defaults -); +const isGotInstance = (value: Got | ExtendOptions): value is Got => is.function_(value); const aliases: readonly HTTPAlias[] = [ 'get', @@ -50,7 +48,7 @@ const create = (defaults: InstanceDefaults): Got => { }); // Got interface - const got: Got = ((url: string | URL | OptionsInit | undefined, options?: OptionsInit, defaultOptions: Options = defaults.options as Options): GotReturn => { + const got: Got = ((url: string | URL | OptionsInit | undefined, options?: OptionsInit, defaultOptions: Options = defaults.options): GotReturn => { const request = new Request(url, options, defaultOptions); let promise: CancelableRequest | undefined; @@ -240,7 +238,7 @@ const create = (defaults: InstanceDefaults): Got => { if (!defaults.mutableDefaults) { Object.freeze(defaults.handlers); - (defaults.options as Options).freeze(); + defaults.options.freeze(); } Object.defineProperty(got, 'defaults', { diff --git a/source/types.ts b/source/types.ts index e589d6182..26fbdd204 100644 --- a/source/types.ts +++ b/source/types.ts @@ -4,7 +4,7 @@ import type {Response} from './core/response.js'; // eslint-disable-next-line import/no-duplicates import type Options from './core/options.js'; // eslint-disable-next-line import/no-duplicates -import type {PaginationOptions, OptionsInit, InternalsType} from './core/options.js'; +import type {PaginationOptions, OptionsInit} from './core/options.js'; import type Request from './core/index.js'; // `type-fest` utilities @@ -18,7 +18,7 @@ export interface InstanceDefaults { /** An object containing the default options of Got. */ - options: InternalsType | Options; + options: Options; /** An array of functions. You execute them directly by calling `got()`.