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

fix: improve TypeScript types #841

Merged
merged 5 commits into from May 29, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
111 changes: 52 additions & 59 deletions @types/index.d.ts
Expand Up @@ -3,10 +3,16 @@
/* eslint-disable no-var, import/no-mutable-exports */

import {Agent} from 'http';
import {AbortSignal} from 'abort-controller';
import Blob from 'fetch-blob';
import * as Blob from 'fetch-blob';
LinusU marked this conversation as resolved.
Show resolved Hide resolved

type HeadersInit = Headers | string[][] | Record<string, string>;
export type AbortSignal = {
readonly aborted: boolean;

addEventListener(type: "abort", listener: (this: AbortSignal, ev: Event) => any, options?: boolean | { passive?: boolean; once?: boolean; }): void;
removeEventListener(type: "abort", listener: (this: AbortSignal, ev: Event) => any, options?: boolean | { capture?: boolean; }): void;
};

export type HeadersInit = Headers | string[][] | Record<string, string>;
SomaticIT marked this conversation as resolved.
Show resolved Hide resolved

/**
* This Fetch API interface allows you to perform various actions on HTTP request and response headers.
Expand All @@ -15,40 +21,38 @@ type HeadersInit = Headers | string[][] | Record<string, string>;
* You can add to this using methods like append() (see Examples.)
* In all methods of this interface, header names are matched by case-insensitive byte sequence.
* */
interface Headers {
append: (name: string, value: string) => void;
delete: (name: string) => void;
get: (name: string) => string | null;
has: (name: string) => boolean;
set: (name: string, value: string) => void;
forEach: (
export class Headers {
constructor(init?: HeadersInit);

append(name: string, value: string): void;
delete(name: string): void;
get(name: string): string | null;
has(name: string): boolean;
set(name: string, value: string): void;
forEach(
callbackfn: (value: string, key: string, parent: Headers) => void,
thisArg?: any
) => void;
): void;

[Symbol.iterator]: () => IterableIterator<[string, string]>;
[Symbol.iterator](): IterableIterator<[string, string]>;
/**
* Returns an iterator allowing to go through all key/value pairs contained in this object.
*/
entries: () => IterableIterator<[string, string]>;
entries(): IterableIterator<[string, string]>;
/**
* Returns an iterator allowing to go through all keys of the key/value pairs contained in this object.
*/
keys: () => IterableIterator<string>;
keys(): IterableIterator<string>;
/**
* Returns an iterator allowing to go through all values of the key/value pairs contained in this object.
*/
values: () => IterableIterator<string>;
values(): IterableIterator<string>;

/** Node-fetch extension */
raw: () => Record<string, string[]>;
raw(): Record<string, string[]>;
}
declare var Headers: {
prototype: Headers;
new (init?: HeadersInit): Headers;
};

interface RequestInit {
export interface RequestInit {
/**
* A BodyInit object or null to set request's body.
*/
Expand Down Expand Up @@ -82,35 +86,39 @@ interface RequestInit {
highWaterMark?: number;
}

interface ResponseInit {
export interface ResponseInit {
headers?: HeadersInit;
status?: number;
statusText?: string;
SomaticIT marked this conversation as resolved.
Show resolved Hide resolved
}

type BodyInit =
export type BodyInit =
| Blob
| Buffer
| URLSearchParams
| NodeJS.ReadableStream
| string;
interface Body {
export interface Body {
readonly body: NodeJS.ReadableStream | null;
readonly bodyUsed: boolean;
readonly size: number;
buffer: () => Promise<Buffer>;
arrayBuffer: () => Promise<ArrayBuffer>;
blob: () => Promise<Blob>;
json: () => Promise<unknown>;
text: () => Promise<string>;

buffer(): Promise<Buffer>;
arrayBuffer(): Promise<ArrayBuffer>;
blob(): Promise<Blob>;
json(): Promise<unknown>;
text(): Promise<string>;
}
declare var Body: {
prototype: Body;
new (body?: BodyInit, opts?: {size?: number}): Body;
};
new(body?: BodyInit, opts?: {size?: number}): Body;
}

export type RequestRedirect = 'error' | 'follow' | 'manual';
SomaticIT marked this conversation as resolved.
Show resolved Hide resolved
export type RequestInfo = string | Body;
SomaticIT marked this conversation as resolved.
Show resolved Hide resolved
export class Request extends Body {
constructor(input: RequestInfo, init?: RequestInit);

type RequestRedirect = 'error' | 'follow' | 'manual';
interface Request extends Body {
/**
* Returns a Headers object consisting of the headers associated with request. Note that headers added in the network layer by the user agent will not be accounted for in this object, e.g., the "Host" header.
*/
Expand All @@ -131,52 +139,37 @@ interface Request extends Body {
* Returns the URL of request as a string.
*/
readonly url: string;
clone: () => Request;
clone(): Request;
}
type RequestInfo = string | Body;
declare var Request: {
prototype: Request;
new (input: RequestInfo, init?: RequestInit): Request;
};

interface Response extends Body {
export class Response extends Body {
constructor(body?: BodyInit | null, init?: ResponseInit);

readonly headers: Headers;
readonly ok: boolean;
readonly redirected: boolean;
readonly status: number;
readonly statusText: string;
readonly url: string;
clone: () => Response;
}

declare var Response: {
prototype: Response;
new (body?: BodyInit | null, init?: ResponseInit): Response;
};

declare function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>;

declare namespace fetch {
function isRedirect(code: number): boolean;
clone(): Response;
}

interface FetchError extends Error {
export class FetchError extends Error {
constructor(message: string, type: string, systemError?: object);

name: 'FetchError';
[Symbol.toStringTag]: 'FetchError';
type: string;
code?: string;
errno?: string;
}
declare var FetchError: {
prototype: FetchError;
new (message: string, type: string, systemError?: object): FetchError;
};

export class AbortError extends Error {
type: string;
name: 'AbortError';
[Symbol.toStringTag]: 'AbortError';
}

export {Headers, Request, Response, FetchError};
export default fetch;
export function isRedirect(code: number): boolean;

export default function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
Copy link
Member

Choose a reason for hiding this comment

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

Since the main field will point to index.cjs, and the types field (which is the corresponding field for the typings) will point to this file, I think that the correct thing here would be to use a CommonJS export:

Suggested change
export default function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
declare function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
export = fetch;

You can read some more info it here: https://github.com/DefinitelyTyped/DefinitelyTyped#a-package-uses-export--but-i-prefer-to-use-default-imports-can-i-change-export--to-export-default

Copy link
Contributor Author

@SomaticIT SomaticIT May 27, 2020

Choose a reason for hiding this comment

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

If you look at the built index.cjs, the resulting exports is the following:

exports.default = fetch;

So the good way to type it is to use export default.
export = fetch is equivalent to module.exports = fetch which is not the case here...

Copy link
Member

Choose a reason for hiding this comment

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

ahh, cool, I was looking at the currently published package which uses module.exports = fetch, my bad!

Copy link
Member

Choose a reason for hiding this comment

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

@SomaticIT I just cloned the repo and made a clean build, and the output in the bundle is module.exports = fetch, thus I think that we should type this as export = fetch

SomaticIT marked this conversation as resolved.
Show resolved Hide resolved
SomaticIT marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions @types/index.test-d.ts
@@ -1,5 +1,6 @@
import {expectType} from 'tsd';
import fetch, {Request, Response, Headers, FetchError, AbortError} from '.';
import AbortController from 'abort-controller';

async function run() {
const getRes = await fetch('https://bigfile.com/test.zip');
Expand Down Expand Up @@ -58,6 +59,11 @@ async function run() {

const response = new Response();
expectType<string>(response.url);

const abortController = new AbortController()
const request = new Request("url", {
signal: abortController.signal
});
}

run().finally(() => {
Expand Down
3 changes: 2 additions & 1 deletion package.json
Expand Up @@ -79,7 +79,8 @@
"lib": [
"es2018"
],
"allowSyntheticDefaultImports": true
"allowSyntheticDefaultImports": false,
"esModuleInterop": false
}
},
"xo": {
Expand Down