Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make types compatible with both DOM and Node.js #543

Merged
merged 1 commit into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/core/Ky.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export class Ky {
this._options.duplex = 'half';
}

this.request = new globalThis.Request(this._input as RequestInfo, this._options as RequestInit);
Copy link
Contributor Author

@alecmev alecmev Nov 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't appear to be needed?

this.request = new globalThis.Request(this._input, this._options);

if (this._options.searchParams) {
// eslint-disable-next-line unicorn/prevent-abbreviations
Expand Down
9 changes: 5 additions & 4 deletions source/types/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export type DownloadProgress = {
totalBytes: number;
};

export type KyHeadersInit = HeadersInit | Record<string, string | undefined>;
// Not HeadersInit directly because @types/node doesn't export it
export type KyHeadersInit = NonNullable<RequestInit['headers']> | Record<string, string | undefined>;

/**
Custom Ky options
Expand Down Expand Up @@ -177,7 +178,7 @@ export type KyOptions = {
const json = await ky('https://example.com', {fetch}).json();
```
*/
fetch?: (input: RequestInfo, init?: RequestInit) => Promise<Response>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is breaking, but actually both @types/node and lib.dom.d.ts define it this way.

fetch?: (input: Input, init?: RequestInit) => Promise<Response>;
};

/**
Expand Down Expand Up @@ -251,8 +252,8 @@ Normalized options passed to the `fetch` call and the `beforeRequest` hooks.
*/
export interface NormalizedOptions extends RequestInit { // eslint-disable-line @typescript-eslint/consistent-type-definitions -- This must stay an interface so that it can be extended outside of Ky for use in `ky.create`.
// Extended from `RequestInit`, but ensured to be set (not optional).
method: RequestInit['method'];
credentials: RequestInit['credentials'];
method: NonNullable<RequestInit['method']>;
credentials: NonNullable<RequestInit['credentials']>;
Comment on lines +255 to +256
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactOptionalPropertyTypes: true fails on this, as optional properties get resolved as Foo | undefined, and that isn't compatible with RequestInit.


// Extended from custom `KyOptions`, but ensured to be set (not optional).
retry: RetryOptions;
Expand Down
4 changes: 2 additions & 2 deletions source/utils/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ export const validateAndMerge = (...sources: Array<Partial<Options> | undefined>
};

export const mergeHeaders = (source1: KyHeadersInit = {}, source2: KyHeadersInit = {}) => {
const result = new globalThis.Headers(source1 as HeadersInit);
const result = new globalThis.Headers(source1 as RequestInit['headers']);
const isHeadersInstance = source2 instanceof globalThis.Headers;
const source = new globalThis.Headers(source2 as HeadersInit);
const source = new globalThis.Headers(source2 as RequestInit['headers']);

for (const [key, value] of source.entries()) {
if ((isHeadersInstance && value === 'undefined') || value === undefined) {
Expand Down
2 changes: 1 addition & 1 deletion test/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ test.serial('works with nullish headers even in old browsers', async t => {
// so we check that Ky never does that and passes an empty object instead.
// See: https://github.com/sindresorhus/ky/issues/260
globalThis.Headers = class Headers extends OriginalHeaders {
constructor(headersInit?: HeadersInit | undefined) {
constructor(headersInit?: RequestInit['headers']) {
t.deepEqual(headersInit, {});
super(headersInit);
}
Expand Down
Loading