Skip to content

[node] Add fetch types #66824

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.

Sign up for GitHub

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

Merged
typescript-bot merged 9 commits into from
Oct 9, 2023
Merged

[node] Add fetch types #66824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
add2448
Add fetch types
andrewbranch Sep 25, 2023
58f95bd
Update tests
andrewbranch Sep 25, 2023
ab70b4c
v18 test workaround
andrewbranch Sep 25, 2023
d73fd65
Revert v18 changes
andrewbranch Sep 26, 2023
ba59420
Remove unnecessary file
andrewbranch Sep 27, 2023
b6a3e0f
Some union order test changes
andrewbranch Sep 28, 2023
6d199df
Fork RequestInit, expose all unidici-RequestInit-referenced types
andrewbranch Oct 2, 2023
491c162
Ensure NodeJS.fetch does not have a value meaning
andrewbranch Oct 2, 2023
e942e80
Remove unnecessary exports
andrewbranch Oct 2, 2023
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
4 changes: 2 additions & 2 deletions types/formidable/formidable-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ http.createServer(async req => {
err;
// $ExpectType Fields<"name" | "age">
fields;
// $ExpectType Files<"avatar" | "document">
// $ExpectType Files<"avatar" | "document"> || Files<"document" | "avatar">
files;
});

Expand All @@ -210,7 +210,7 @@ http.createServer(async req => {
const [newFields, newFiles] = await form.parse<"name" | "age", "avatar" | "document">(req); // testing with promise and type arguments
// $ExpectType Fields<"name" | "age">
newFields;
// $ExpectType Files<"avatar" | "document">
// $ExpectType Files<"avatar" | "document"> || Files<"document" | "avatar">
newFiles;
});

Expand Down
4 changes: 2 additions & 2 deletions types/hidefile/test/hidefile-tests.cjs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import hidefile = require("hidefile");

hidefile.hide("./", () => {}); // $ExpectType void
hidefile.hideSync("./"); // $ExpectType string | Buffer | URL
hidefile.hideSync("./"); // $ExpectType string | Buffer | URL || string | URL | Buffer
hidefile.isDotPrefixed("./"); // $ExpectType boolean
hidefile.isHidden("./", () => {}); // $ExpectType void
hidefile.isHiddenSync("./"); // $ExpectType boolean
hidefile.reveal("./", () => {}); // $ExpectType void
hidefile.revealSync("./"); // $ExpectType string | Buffer | URL
hidefile.revealSync("./"); // $ExpectType string | Buffer | URL || string | URL | Buffer
hidefile.shouldBeHidden("./", () => {}); // $ExpectType void
hidefile.shouldBeHiddenSync("./"); // $ExpectType boolean
2 changes: 1 addition & 1 deletion types/levelup/levelup-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ db.isOpen();
// $ExpectType boolean
db.isClosed();

// $ExpectType "closed" | "open" | "opening" | "new" | "closing"
// $ExpectType "closed" | "open" | "opening" | "new" | "closing" || "open" | "closed" | "opening" | "new" | "closing"
Comment thread
andrewbranch marked this conversation as resolved.
db.status;

// $ExpectType boolean
Expand Down
58 changes: 58 additions & 0 deletions types/node/globals.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,62 @@ declare namespace NodeJS {
interface ReadOnlyDict<T> {
readonly [key: string]: T | undefined;
}

namespace fetch {
type _Request = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Request;
type _Response = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Response;
type _FormData = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").FormData;
type _Headers = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Headers;
type _RequestInit = typeof globalThis extends { onmessage: any } ? {}
: import("undici-types").RequestInit;
type Request = globalThis.Request;
type Response = globalThis.Response;
type Headers = globalThis.Headers;
type FormData = globalThis.FormData;
type RequestInit = globalThis.RequestInit;
type RequestInfo = import("undici-types").RequestInfo;
type HeadersInit = import("undici-types").HeadersInit;
type BodyInit = import("undici-types").BodyInit;
type RequestRedirect = import("undici-types").RequestRedirect;
type RequestCredentials = import("undici-types").RequestCredentials;
type RequestMode = import("undici-types").RequestMode;
type ReferrerPolicy = import("undici-types").ReferrerPolicy;
type Dispatcher = import("undici-types").Dispatcher;
type RequestDuplex = import("undici-types").RequestDuplex;
}
}

interface RequestInit extends NodeJS.fetch._RequestInit {}

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

interface Request extends NodeJS.fetch._Request {}
declare var Request: typeof globalThis extends {
onmessage: any;
Request: infer T;
} ? T
: typeof import("undici-types").Request;

interface Response extends NodeJS.fetch._Response {}
declare var Response: typeof globalThis extends {
onmessage: any;
Response: infer T;
} ? T
: typeof import("undici-types").Response;

interface FormData extends NodeJS.fetch._FormData {}
declare var FormData: typeof globalThis extends {
onmessage: any;
FormData: infer T;
} ? T
: typeof import("undici-types").FormData;

interface Headers extends NodeJS.fetch._Headers {}
declare var Headers: typeof globalThis extends {
onmessage: any;
Headers: infer T;
} ? T
: typeof import("undici-types").Headers;
3 changes: 3 additions & 0 deletions types/node/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,8 @@
"types": "index",
"typesVersions": {
"<=4.8": { "*": ["ts4.8/*"] }
},
"dependencies": {
"undici-types": "~5.25.1"
Copy link
Copy Markdown
Member Author

@andrewbranch andrewbranch Sep 25, 2023

Choose a reason for hiding this comment

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

This is the first version of undici-types available, but it reflects a newer minor version than is actually included in Node.js so far. It sounded from nodejs/undici#2261 (comment) like we will be able to wait on a new Node.js version comes out containing undici 5.25 and bump the @types/node version accordingly if we want this dependency mapping to be perfectly accurate. cc @Ethan-Arrowood

Copy link
Copy Markdown
Contributor

@Ethan-Arrowood Ethan-Arrowood Sep 25, 2023

Choose a reason for hiding this comment

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

Yeah, I don't think there's much of an issue shipping this now and then getting synced up later. If there's an actual issue (some bug or whatever), then just tell me exactly which version you want me to publish and I'll manually go a publish the undici-types version.

Copy link
Copy Markdown
Contributor

@Semigradsky Semigradsky Sep 29, 2023

Choose a reason for hiding this comment

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

JFI, the latest undici version for Node.js versions:

Copy link
Copy Markdown
Contributor

@SimonSchick SimonSchick Oct 5, 2023

Choose a reason for hiding this comment

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

Kinda concerned about the impact of this on users of @types/node I've already seen a ton of duplicate installations of the package with different versions, using a restrictive version range for undici is likely going to make this worse.

Copy link
Copy Markdown
Member Author

@andrewbranch andrewbranch Oct 5, 2023

Choose a reason for hiding this comment

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

Can you elaborate? This isn’t undici, just undici-types.

Copy link
Copy Markdown
Contributor

@mcollina mcollina Oct 7, 2023

Choose a reason for hiding this comment

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

All those patch versions are identical, minus tiny bugfixes for other issues.

Copy link
Copy Markdown
Contributor

@SimonSchick SimonSchick Oct 7, 2023

Choose a reason for hiding this comment

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

More curious if we should make this >=5.22.1 to allow users squash/consolidate their dependencies more easily, a lot of libraries currently depend on @types/node@* to avoid this already, some don't and it causes multiple instances of @types/node to be installed which sometimes have been conflicting.

I'm not sure if I'm overstating the issue of if this is simply not a concern.

Copy link
Copy Markdown
Member

@jakebailey jakebailey Oct 9, 2023

Choose a reason for hiding this comment

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

Yeah, mainly I assume this is a question of "what if there are multiple types packages in a project", where previously @types/node had no deps so could only "conflict" with itself in a compatible fashion, but adding a dep expands that risk (where restricting the version range increases that risk). I'm not sure that it will actually cause a problem, though.

Copy link
Copy Markdown
Member Author

@andrewbranch andrewbranch Oct 9, 2023

Choose a reason for hiding this comment

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

The downside to loosening the range is indicating to a Node.js 20.3.X user (for example) that they have access to undici features that they actually don’t. undici just recently added support for HTTP2, so using a >= range could have incorrectly led users of a Node.js that hadn’t adopted that undici version to try to access those features, had this PR happened earlier.

On the other hand, I think you’re just automatically screwed if you have two @types/node versions in the same program, even without dependencies.

}
}
32 changes: 32 additions & 0 deletions types/node/test/globals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,35 @@ declare var RANDOM_GLOBAL_VARIABLE: true;
x.reason; // $ExpectType any
x.throwIfAborted(); // $ExpectType void
}

// fetch
{
// This tsconfig.json references lib.dom.d.ts. The fetch
// types included in globals.d.ts are designed to be empty
// merges when lib.dom.d.ts is included. This test ensures
// the merge works, but the types observed are from lib.dom.d.ts.
fetch("https://example.com").then(response => {
response.arrayBuffer(); // $ExpectType Promise<ArrayBuffer>
response.blob(); // $ExpectType Promise<Blob>
response.formData(); // $ExpectType Promise<FormData>

// undici-types uses `Promise<unknown>` for `json()`
// This $ExpectType will change if tsconfig.json drops
// lib.dom.d.ts.
response.json(); // $ExpectType Promise<any>
response.text(); // $ExpectType Promise<string>
});
const fd = new FormData();
fd.append("foo", "bar");
const headers = new Headers();
headers.append("Accept", "application/json");
fetch("https://example.com", { body: fd });

fetch(new URL("https://example.com"), {
// @ts-expect-error this should not be available when lib.dom.d.ts is present
dispatcher: undefined,
});

// @ts-expect-error
NodeJS.fetch;
}
58 changes: 58 additions & 0 deletions types/node/ts4.8/globals.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,62 @@ declare namespace NodeJS {
interface ReadOnlyDict<T> {
readonly [key: string]: T | undefined;
}

namespace fetch {
type _Request = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Request;
type _Response = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Response;
type _FormData = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").FormData;
type _Headers = typeof globalThis extends { onmessage: any } ? {} : import("undici-types").Headers;
type _RequestInit = typeof globalThis extends { onmessage: any } ? {}
: import("undici-types").RequestInit;
type Request = globalThis.Request;
type Response = globalThis.Response;
type Headers = globalThis.Headers;
type FormData = globalThis.FormData;
type RequestInit = globalThis.RequestInit;
type RequestInfo = import("undici-types").RequestInfo;
type HeadersInit = import("undici-types").HeadersInit;
type BodyInit = import("undici-types").BodyInit;
type RequestRedirect = import("undici-types").RequestRedirect;
type RequestCredentials = import("undici-types").RequestCredentials;
type RequestMode = import("undici-types").RequestMode;
type ReferrerPolicy = import("undici-types").ReferrerPolicy;
type Dispatcher = import("undici-types").Dispatcher;
type RequestDuplex = import("undici-types").RequestDuplex;
}
}

interface RequestInit extends NodeJS.fetch._RequestInit {}

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

interface Request extends NodeJS.fetch._Request {}
declare var Request: typeof globalThis extends {
onmessage: any;
Request: infer T;
} ? T
: typeof import("undici-types").Request;

interface Response extends NodeJS.fetch._Response {}
declare var Response: typeof globalThis extends {
onmessage: any;
Response: infer T;
} ? T
: typeof import("undici-types").Response;

interface FormData extends NodeJS.fetch._FormData {}
declare var FormData: typeof globalThis extends {
onmessage: any;
FormData: infer T;
} ? T
: typeof import("undici-types").FormData;

interface Headers extends NodeJS.fetch._Headers {}
declare var Headers: typeof globalThis extends {
onmessage: any;
Headers: infer T;
} ? T
: typeof import("undici-types").Headers;
22 changes: 22 additions & 0 deletions types/node/ts4.8/test/globals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,25 @@ declare var RANDOM_GLOBAL_VARIABLE: true;
x.reason; // $ExpectType any
x.throwIfAborted(); // $ExpectType void
}

// fetch
{
fetch("https://example.com").then(response => {
response.arrayBuffer(); // $ExpectType Promise<ArrayBuffer>
response.blob(); // $ExpectType Promise<Blob>
response.formData(); // $ExpectType Promise<FormData>
response.json(); // $ExpectType Promise<unknown>
response.text(); // $ExpectType Promise<string>
});
const fd = new FormData();
fd.append("foo", "bar");
const headers = new Headers();
headers.append("Accept", "application/json");
fetch("https://example.com", { body: fd });
fetch(new URL("https://example.com"), {
dispatcher: undefined,
});

// @ts-expect-error
NodeJS.fetch;
}