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

node@18.x.x version not fetch global api ? #60924

Closed
xiaocai-rookie opened this issue Jun 23, 2022 · 135 comments · Fixed by #66824
Closed

node@18.x.x version not fetch global api ? #60924

xiaocai-rookie opened this issue Jun 23, 2022 · 135 comments · Fixed by #66824

Comments

@xiaocai-rookie
Copy link

I use @types/node@18.0.0 version ,but use fetch fail,typescript to me "fetch is not defined"
Could I know , why "fetch is not defined" ?

@mahdi-farnia
Copy link

I think because it's still experimental
Also definition for node:readline/promises is not available

@xiaocai-rookie
Copy link
Author

I think because it's still experimental Also definition for node:readline/promises is not available

Yes, node official announce too.

@shadow-light
Copy link

It was simply not added because of time constraints so hopefully sometime soon...

@SuperchupuDev
Copy link

@SimonSchick any chance we can get fetch added anytime soon? it's a really important thing to have in the types

@piranna
Copy link

piranna commented Jul 19, 2022

Is it available in any branch or development version we can make use of?

@piranna
Copy link

piranna commented Jul 21, 2022

LOL! Just use vanilla js that solve the problem.

I would love to, but my client asked me to use Typescript :-)

@SimonSchick
Copy link
Contributor

The problem is still that dom libs are not modularized, if they were node typings could just reference dom specs.
As this time I'd rather not copy & paste the API definitions into node as that could create future liabilities.

@piranna
Copy link

piranna commented Jul 21, 2022

The problem is still that dom libs are not modularized, if they were node typings could just reference dom specs.
As this time I'd rather not copy & paste the API definitions into node as that could create future liabilities.

Agree on that. Can you point us where it's defined so we can copy it ourselves in our projects, meanwhile it get added to Node.js definitions?

@mahdi-farnia
Copy link

mahdi-farnia commented Jul 21, 2022

I think it's better to not to use fetch Api in node. But if U need it ( for any reason ) it's seems easy to define one:

declare function fetch(...): ...;
  1. open a new file ( js or ts ) that fetch already available
  2. Command + Click or Ctrl + Click on fetch ( goto definition )
  3. Copy and paste the types to source file

If you dont wanna see it in your ts files you can just simply create types.d.ts file ( name is important ) or create a folder @types ( name is important ) and put it on that folder

Note: your d.ts ( definition file ) must not be a module if it is, you have to import the fetch type in every file that you are using it.

@piranna
Copy link

piranna commented Jul 21, 2022

I think it's better to not to use fetch Api in node

Why not?

simply create types.d.ts file

That's what I'm asking for.

@mahdi-farnia
Copy link

I think it's better to not to use fetch Api in node

Why not?

simply create types.d.ts file

That's what I'm asking for.

  1. It's experimental
  2. Node@18 is not even the LTS version, it's the latest, not the most stable one

@piranna
Copy link

piranna commented Jul 21, 2022

Not a good code indeed : nodejs/undici

I would not say @mcollina code is not good, with so much performance obsesive code done...

But not worst than the microsoft average.
Good luck for this : https://github.com/nodejs/node/tree/v18.x/lib

Node.js is not from Microsoft, Microsoft has only developed Typescript on top of it.

@piranna
Copy link

piranna commented Jul 21, 2022

  1. It's experimental

Good point, fair enough.

2. Node@18 is not even the LTS version, it's the latest, not the most stable one

It's the most stable one, just only doesn't have yet the LTS denomination. I could agree that odd versions (v17, v19...) are unstable development versions and should not be used on production, but pair ones (v16, v18) is totally fine.

@rjoonas
Copy link
Contributor

rjoonas commented Aug 10, 2022

Yeah, node 18 will be LTS in two months (2022-10-25).

The problem is still that dom libs are not modularized, if they were node typings could just reference dom specs. As this time I'd rather not copy & paste the API definitions into node as that could create future liabilities.

@SimonSchick, so are we expecting lib.dom to be modularized before we can add node typings for fetch? Sounds like something that might take a while.

I wonder if in the meanwhile we could add fetch to types/node by copy-pasting fetch, Request, Response, Headers etc. definitions as a standalone module that would be easy to delete when it's possible to just reference the respective DOM definitions.

@piranna
Copy link

piranna commented Aug 10, 2022

I wonder if in the meanwhile we could add fetch to types/node by copy-pasting fetch, Request, Response, Headers etc. definitions as a standalone module that would be easy to delete when it's possible to just reference the respective DOM definitions.

Agree, and add a TODO: comment about that fact, or maybe also store them in an independent types file and reference it from the lib.dom types... starting with the modularization ourselves :-)

@piranna
Copy link

piranna commented Aug 10, 2022

Or just import the lib.dom types, and re-export the ones we are interested about.

@brielov
Copy link

brielov commented Aug 14, 2022

Any news on this? I'd prefer not to have my global namespace polluted with lib.dom stuff.

@ImRodry
Copy link
Contributor

ImRodry commented Aug 28, 2022

The problem is still that dom libs are not modularized, if they were node typings could just reference dom specs. As this time I'd rather not copy & paste the API definitions into node as that could create future liabilities.

Is there any chance you could install undici as a dep on @types/node and re-export the fetch types from it @SimonSchick?
Also does anyone know if the fetch API will be considered stable by the time Node v18 hits LTS?

@mcollina
Copy link
Contributor

We are not planning to have a stable fetch for when v18 hits LTS.

@llwwbb
Copy link
Contributor

llwwbb commented Aug 30, 2022

We are not planning to have a stable fetch for when v18 hits LTS.

I don't quite understand whether node18 is lts or not, and whether fetch is experimental or not, what does it have to do with whether there is a definition of fetch in @types/node? Has anyone stipulated that all the apis defined in @types are stable? I do not think so. @types is just a type definition. Anything that can be called or accessed in js should have a corresponding type definition, regardless of whether it is experimental or not.

@piranna
Copy link

piranna commented Aug 30, 2022

I'm not so much concerned about being stable or not... Just only, i think if it's available without flags, It should be available for the same version. It's said, @types/node 18.0.0 should have all the types of Node.js 18, i can be happy of not having available a newer version or being It delayed until it's complete and on pair, and need to use and stick with @types/node 17.x.x, than install 18.x.x thinking It has all the same types and have some of them missing. At least, if It will provide only non-experimental ones, notify It in the readme.

@HoldYourWaffle
Copy link
Contributor

Or just import the lib.dom types, and re-export the ones we are interested about.

This wouldn't work because merely touching lib.dom.d.ts pollutes the global scope.

However, as a temporary workaround you could add a declaration file to your project that re-exports the types from @types/node-fetch:

import _fetch from 'node-fetch'

declare global {
    declare var fetch: typeof _fetch;
}

node-fetch'types aren't 100% accurate for native fetch of course, but depending on your preferences it could be a good workaround compared to having no types at all.

@piranna
Copy link

piranna commented Sep 5, 2022

I like the workaround, thanks

@thw0rted
Copy link
Contributor

thw0rted commented Sep 5, 2022

I just noticed that this issue does not have a link to microsoft/TypeScript-DOM-lib-generator#1207 . As @HoldYourWaffle pointed out, the current lib-dom does not export types, it only makes global declarations. It would be great if those types were available for "clean" import, but it doesn't seem to be a priority yet. Maybe an upvote for that issue would help to solve this one?

@piranna
Copy link

piranna commented Sep 5, 2022

I just noticed that this issue does not have a link to microsoft/TypeScript-DOM-lib-generator#1207 . As @HoldYourWaffle pointed out, the current lib-dom does not export types, it only makes global declarations. It would be great if those types were available for "clean" import, but it doesn't seem to be a priority yet. Maybe an upvote for that issue would help to solve this one?

Upvoted and commented at microsoft/TypeScript-DOM-lib-generator#1207 (comment)

@Qix-
Copy link

Qix- commented Sep 14, 2022

What does experimental status, DOM modularity, or LTS have anything to do with whether or not types are included here? I don't see the point in arguing any of that - the point is, it exists in Node 18 without a flag. There should be types for it. No, sorry, DOM is not acceptable because Node does not use the DOM. WhatWG Fetch is a standard pulled out of the browsers and re-implemented in Node.js because it is useful. The fact it originated in browsers is completely irrelevant.

Why is this a discussion? This is a hole in the types, plain and simple.

@thw0rted
Copy link
Contributor

The reason I brought up DOM modularity is basically #59905 . Lots of people wind up with the @types/node included in their frontend project, or lib: ['dom'] in their backend project, for a wide variety of reasons. If @types/node ships a global fetch, lots of users will suddenly find that they are merging those types with the ones built into lib-dom. I'm not sure what the result will be, or even how to test it, but this impedance-mismatch has caused problems before and I suspect they're about to get a lot worse.

@thw0rted
Copy link
Contributor

Also while I'm thinking of it, re @HoldYourWaffle 's comment about adding your own types for this, I wanted to suggest

declare global {
  var fetch: typeof import("node-fetch").default;
}

I believe this makes it clear to the typechecker that you're 100% not trying to import node-fetch at runtime, and only care about the types.

Of course I do still think we need a path to correct types directly in @types/node, I'm just not sure what that's going to look like.

@Qix-
Copy link

Qix- commented Sep 14, 2022

Node.js uses undici under the hood for fetch. Not sure if it's exposed directly or if it's wrapped, but it comes with Typescript types that could be consumed here somehow.

EDIT: exposed directly (see #60924 (comment))

@thw0rted
Copy link
Contributor

thw0rted commented Sep 1, 2023

This probably isn't the place to discuss it, but my first thought on reading your opening paragraph was, has the TS team considered taking on a "lib.node.d.ts" so they can be in charge of ensuring conflicts don't happen? It seems precarious to put so much responsibility in the hands of a community maintained library. As you point out, it's probably a rare frontend project that doesn't wind up with @types/node installed eventually.

@pauldraper
Copy link

@thw0rted if anything, the team is moving away from the bundled type declarations.

https://www.npmjs.com/package/@types/web

Though that is a somewhat different question than maintainence.

@andrewbranch
Copy link
Member

@thw0rted I think both the collaboration between the TS team and the community on @types/node and the infrastructure overlap between TS and DT are working well (age of this issue notwithstanding), and there would be little to gain by moving the work into the TypeScript repo. I think we would lose a lot, actually.

@andrewbranch
Copy link
Member

I think I have both a version that copies undici types and a version that has undici as a dependency working. The latter is satisfyingly short:

declare namespace NodeJS {
  namespace undici {
    type Request = typeof globalThis extends { onmessage: any } ? {} : import("undici").Request;
    type Response = typeof globalThis extends { onmessage: any } ? {} : import("undici").Response;
    type File = typeof globalThis extends { onmessage: any } ? {} : import("undici").File;
    type FormData = typeof globalThis extends { onmessage: any } ? {} : import("undici").FormData;
    type Headers = typeof globalThis extends { onmessage: any } ? {} : import("undici").Headers;
  }
}

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

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

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

declare var File: typeof globalThis extends {
  onmessage: any;
  File: infer T;
} ? T : typeof import("undici").File;
interface File extends NodeJS.undici.File {}

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

declare var Headers: typeof globalThis extends {
  onmessage: any;
  Headers: infer T;
} ? T : typeof import("undici").Headers;
interface Headers extends NodeJS.undici.Headers {}

It works well for declaring the value side of Request, Response, Headers, and FormData, but the type sides of Request, Response, and FormData don’t merge cleanly with the DOM interfaces. I think the conditional type trick can be used deeper within those types to fix the merge, but the location of the changes would be in the undici types themselves.

Upon taking a closer look at how @thw0rted made the Blob global work, I realized I had missed something, so it does appear to be possible to use undici unchanged if that’s the route we want to go.

The same conditional type trick doesn’t work for declaring fetch itself... @types/node could... add an overload instead of changing the type... Depending on the overload order, this would be non-ideal either for actual Node.js projects or for frontend projects who accidentally have @types/node included—someone is going to get the wrong overload. But with Request and Response being properly substituted, this might not be a huge deal.

Yeah. The signatures are identical with the other globals substituted, so adding the overload works perfectly fine.

Planning to chat with the rest of the team tomorrow about which approach to PR.

@andrewbranch
Copy link
Member

The feedback I got from the team was that we should try to avoid the undici-as-dependency approach mostly due to the significant impact it would have on @types/node total installation size. It was suggested that we could make a @types/undici rather than copying them into @types/node itself, which would make it easier for undici itself to drop their included types if they’re interested in doing that.

@Qix-
Copy link

Qix- commented Sep 11, 2023

Well DT is good for packages that don't have types already but for packages that do, or are otherwise written in TS, DT is usually the worse of the two options due to desynchronization. If there could be a way to extract types at build time, prior to publishing, that'd be the 'ideal' way depending on how you look at things.

@piranna
Copy link

piranna commented Sep 12, 2023

Prepublish script?

@jchitel
Copy link

jchitel commented Sep 28, 2023

It seems to me that this points to a fundamental problem with how TypeScript handles type definitions, particularly with global types. The actual TypeScript team should be more involved in this discussion.

I think the best solution today is to use a clever combination of the lib and types settings within multiple tsconfig files in a single project.

An example web project might have 3 "environments" that are used by various files: a "web" environment for production code that will run in a browser, a "test" environment that runs in Node (or Deno or Bun) and polyfills web APIs using something like happy-dom or jsdom, and a "server" environment that includes everything running in a regular Node (or Deno or Bun) environment with no web APIs.

This project can handle the differences in available global APIs and modules by defining 3 separate tsconfig files with explicitly defined lib and types settings:

  • tsconfig.web.json:
    • "lib": ["dom", "dom.iterable", "..."]
    • "types": [] (explicitly declare no types to be included)
    • "include": ["./client"]
    • "exclude": ["./client/**/__tests__"] (exclude tests, which are covered below)
  • tsconfig.node.json:
    • "lib": ["..."] (no DOM APIs are available)
    • "types": ["node"] (explicitly include only Node types)
    • "include": ["./server", "..."] (add whatever other files run in Node, like webpack.config.ts or vite.config.ts)
  • tsconfig.test.json:
    • "lib": ["dom", "dom.iterable", "..."]
    • "types": ["node"] (explicitly include only Node types)
    • "include": ["./client/**/__tests__"] (only include tests)

This solves most of the problem, notably for files that run only on the web or only in Node. However, the "test" environment has both DOM and Node types. Technically we can't know which fetch types are correct, since any DOM polyfill that is used may or may not overwrite Node's built-in fetch implementation.

TypeScript's solution for this problem up to this point has been to merge both definitions into one. This works well for interfaces, which will simply accept properties from both definitions. It does not work well for functions, however. We already see this problem with setTimeout. The DOM types return a number, while the Node types return a NodeJS.Timeout. The simple workaround on the web side is to use window.setTimeout, which resolves to only the DOM version. But there is no good solution on the Node side. In the case of setTimeout things typically just happen to work, but if there was a more complicated difference between the types, this would have been a problem ages ago.

As more and more web APIs are added to non-browser environments, this problem is likely to only get worse. For example, since fetch was designed to work in the browser sandbox, it supports only a subset of HTTP (e.g. no direct access to all cookies), so some environments add additional properties to the options object passed to fetch. Other web APIs are likely to have similar situations.

We need the ability to more finely tune what an environment looks like, down to selecting individual items from various type definitions. This might be possible using conditional types and Pick and Omit, but those don't really feel "first-class" IMO. I'd like to hear what other people's thoughts are on this.

@jchitel
Copy link

jchitel commented Sep 28, 2023

The actual TypeScript team should be more involved in this discussion.

😕

Oops! I didn't notice that someone from the TypeScript team was already here! I read the earlier comments more thoroughly than the latter ones, so I completely missed your responses. I'm sorry if I offended anyone.

@andrewbranch
Copy link
Member

No worries 😄 I think you’ll find some of those links interesting. I think this issue is probably not the best place to go deep into the weeds on rethinking environmental types from the ground up. I broadly agree with you, and this problem used to be very much top-of-mind for me. But from my perspective, each time we’ve hit a wave of global type clashes that seems really intractable, we’ve actually been able to resolve things pretty well with the tools we already have. The PR I have up to add fetch typings, closing this issue, is an example. It’s not perfect but it’s really close, I think. Another one:

In the case of setTimeout things typically just happen to work

Again, it’s not perfect, but it doesn’t provide a super strong justification for overhauling the way global types work. We just try our best, through DT contributions, to discourage Node.js-related typings from pulling in the DOM unnecessarily and vice versa, so users can hopefully avoid having @types/node and lib.dom.d.ts in the same compilation, while ensuring they’re written in a way that doesn’t explode into errors when they are in the same compilation. And this continues to mostly work out for most cases.

I would love to have a more rigorous solution in the future, but it’s definitely not worth holding off on adding fetch while we wait for a silver bullet.

@jchitel
Copy link

jchitel commented Sep 28, 2023

That makes a lot of sense. The Issues you posted are super interesting and show that others have clearly thought more about this than I have 😅

I agree that there needs to be a solution right now to resolve this particular issue. I'll defer further discussion to your PR.

@joshxyzhimself
Copy link

For now undici works.

npm install undici
import { fetch } from "undici";

@andrewbranch
Copy link
Member

#66824 is ready. It only updates v20 typings due to a test infrastructure limitation that will likely be resolved within a couple weeks. I’d like to wait for a non-TS team @types/node maintainer to approve.

@bootrino
Copy link

bootrino commented May 6, 2024

This is a giant thread and all i want is to know how to use the correct types for native fetch in nodejs.

Does this example show the correct way to do types with node native fetch?

const nodeNativeFetch = async (url: string, options: RequestInit = {}): Promise<globalThis.Response> => {
    const response: globalThis.Response = await fetch(url, options);
    const headers: Headers = response.headers;

    // Log each header
    for (let pair of headers.entries()) {
        const key: string = pair[0];
        const value: string = pair[1];
        console.log(`${key}: ${value}`);
    }

    return response;
};

@thw0rted
Copy link
Contributor

thw0rted commented May 6, 2024

I believe fetch types are pretty good on the latest @types/node@20. Your example looks right to me. You should not need to reference "globalThis" -- just Promise<Response> should typecheck fine -- but it might not be a bad idea if you want to make it very clear that you aren't referring to import { Response } from 'express'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.