Skip to content

Commit

Permalink
General code improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak committed Jul 30, 2021
1 parent 732e9bd commit a5dd9aa
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 72 deletions.
6 changes: 3 additions & 3 deletions source/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> = new Set(['GET', 'HEAD']);
Expand Down Expand Up @@ -125,7 +125,7 @@ const proxiedRequestEvents = [
'continue',
'information',
'upgrade',
];
] as const;

const noop = () => {};

Expand Down
80 changes: 19 additions & 61 deletions source/core/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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];
}
}
}
Expand Down Expand Up @@ -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);
2 changes: 1 addition & 1 deletion source/core/utils/proxy-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {EventEmitter} from 'events';
type Fn = (...args: unknown[]) => void;
type Fns = Record<string, Fn>;

export default function proxyEvents(from: EventEmitter, to: EventEmitter, events: string[]): () => void {
export default function proxyEvents(from: EventEmitter, to: EventEmitter, events: Readonly<string[]>): () => void {
const eventFunctions: Fns = {};

for (const event of events) {
Expand Down
8 changes: 3 additions & 5 deletions source/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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;

Expand Down Expand Up @@ -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', {
Expand Down
4 changes: 2 additions & 2 deletions source/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()`.
Expand Down

0 comments on commit a5dd9aa

Please sign in to comment.