lquixada / cross-fetch

Universal WHATWG Fetch API for Node, Browsers and React Native.
MIT License
1.67k stars 104 forks source link

Typings in index.d.ts cause dom types to be available globally in any package using cross-fetch #70

Closed tupelo-schneck closed 3 years ago

tupelo-schneck commented 4 years ago

Since index.d.ts contains /// <reference lib="dom" />, all the lib.dom types are imported globally for anyone who uses cross-fetch: https://github.com/microsoft/TypeScript/issues/33901

That means if I'm working in a TypeScript project where I don't want to have lib.dom available (which is surely true of anyone using cross-fetch) I have to somehow override the cross-fetch typings.

I suggest depending on @types/node-fetch instead and using

export * from 'node-fetch';
export { default } from 'node-fetch';

to re-export the declarations from it.

jstewmon commented 4 years ago

I came here after running into this issue: https://github.com/prisma-labs/graphql-request/issues/206

Referencing the TS dom lib in packages which also target node causes problems...

https://github.com/DefinitelyTyped/DefinitelyTyped/issues/12044#issuecomment-331375183

In addition to the disjoint set of globals, there are also conflicting global types, like the return types of timer functions.

calebfrieze commented 3 years ago

Any movement on this? Ended up moving to TypeScript on a node.js project that uses this library.RequestInit gives me an error when i pass in an agent stating that agent does not exist on RequestInit. Seems like this is also due to the fact that your index.d.ts contains /// <reference lib="dom" />.

carlpaten commented 3 years ago

@lquixada would you welcome a PR addressing this?

lquixada commented 3 years ago

@lilred sure! The ideal PR would be all the type definitions for fetch, Headers, Request and Response from this document: https://github.com/microsoft/TypeScript/blob/master/lib/lib.dom.d.ts.

jstewmon commented 3 years ago

👋 Hi, after running into this issue several times across various projects, I recently wrote a script to automate the extraction of dom lib types for the SimpleWebAuthn project.

In my [minuscule] free time, I've been working on making a standalone CLI out of it in case other projects wanted to take a similar approach. I just published an initial version under the package name ts-graft. It is still very basic, but it is functional and I believe can make the task of extracting type definitions from lib files much easier than doing so by hand.

I'm not necessarily trying to promote the project - I've barely had any time to work on it as is, but if you want to try it out for this, I think it will make the chore very easy.

Of course, if you do have any feedback on what I've got, it's welcome. 🤗

lquixada commented 3 years ago

hey @jstewmon! this tool looks amazing! will def take a look at it! Thanks!

lquixada commented 3 years ago

Closing this as it was addressed on PR #93 .

mshwery commented 3 years ago

Noting here that the closing PR got reverted, so this issue still stands.

If the type namespaces match, can we avoid using /// <reference lib="dom" /> altogether? Consumers should already be including lib: ['dom'] as needed in browser-based targets, while excluding it in Node targets (since it's inaccurate and can cause type conflicts).

In the browser tsconfig, the declarations should merge, right?

bali182 commented 2 years ago

Hello, just came across this issue. It's still present in 3.1.5.

May I ask why @jstewmon's PR was reverted?

We could use lib: ['dom'], but if this is used in a node+typescript project, that kind of defeats the purpose of using typescript, as now all usages of browser apis will compile, even though they will fail at runtime.

ab-pm commented 11 months ago

Please reopen after 3fce389185b12317169e445931f5c724a73ccc26 due to #101

ladal1 commented 3 weeks ago

The 4.0.0 version still contains the dom import which can cause errors when used casually with node, such reference is too strong for use in types right now. This cange needs to be reviewed again