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

Fetch types are incompatible with "lib/dom" definitions #1943

Open
kettanaito opened this issue Feb 20, 2023 · 7 comments
Open

Fetch types are incompatible with "lib/dom" definitions #1943

kettanaito opened this issue Feb 20, 2023 · 7 comments
Labels
bug Something isn't working Types Changes related to the TypeScript definitions

Comments

@kettanaito
Copy link
Contributor

kettanaito commented Feb 20, 2023

Bug Description

The Request interface exposed by Undici does not satisfy the same type from the global type definitions (lib/dom), and also missing properties according to the Fetch API specification:

export declare class Request implements BodyMixin {

Reproducible By

As an example, consider the following code:

import { Request } from 'undicit'

function transform(req: globalThis.Request) {}

transform(new Request('/hello'))
// ^^^ Property 'referrer' is missing in type 'import(".../node_modules/undici/types/fetch").Request' but required in type 'Request'.ts(2345)

The referrer property is not present in the Undici's Request type definition while it's both required in lib/dom definitions and the Fetch API specification.

readonly attribute [USVString](https://webidl.spec.whatwg.org/#idl-USVString) [referrer](https://fetch.spec.whatwg.org/#dom-request-referrer);

Expected Behavior

Fetch API primitives such as Request, Response and Headers, are compatible with the global

Logs & Screenshots

No applicable logs.

Environment

  • Node: v16.16.0
  • Undici: ^5.20.0
  • TypeScript: ^4.9.4

Additional context

I understand that I'm setting lib/dom as the source of truth here but I expect that shouldn't matter since:

  1. Both Undici and lib/node aim to implement the Fetch API specification.
  2. Undici is objectively missing some properties in its type definitions defined in the spec, like referrer.

Also, for context, I'm spotting this while building Interceptors (mswjs/interceptors#340) where I try using Undici as a Fetch polyfill to guarantee fetch primitives can be created in Node versions prior to global fetch.

I'd like to open a pull request to ensure this compliance after discussing this with the maintainers of this lib.

@kettanaito kettanaito added the bug Something isn't working label Feb 20, 2023
@kettanaito kettanaito changed the title Fetch types are incompatible with TypeScript Fetch types are incompatible with "lib/dom" definitions Feb 20, 2023
@KhafraDev KhafraDev added the Types Changes related to the TypeScript definitions label Feb 20, 2023
@KhafraDev
Copy link
Member

Would you like to send in a PR to add the missing propert(y/ies) to Request?

@kettanaito
Copy link
Contributor Author

@KhafraDev, absolutely! Will open a pull request tomorrow, we can follow up there. Thanks for a quick response on this.

@kettanaito
Copy link
Contributor Author

Hey, @KhafraDev. I've opened a draft to improve typings of Undici. Would appreciate an early review of this and any potential discussion. Thanks.

@sharifzadesina
Copy link

sharifzadesina commented Oct 19, 2023

It is interesting that this bug is not yet fixed. Can we do any help?

smaye81 added a commit to connectrpc/examples-es that referenced this issue Oct 19, 2023
This updates all dependencies in the repo.

Note: Svelte was reverted because the Undici update breaks tsc
compilation due to an issue with Undici's types for `Request` and our
usage of a custom fetch. See issue here:
nodejs/undici#1943
@mcollina
Copy link
Member

It is interesting that this bug is not yet fixed. Can we do any help?

PRs are welcome!

mmso added a commit to ProtonMail/WebClients that referenced this issue Nov 7, 2023
@NatoBoram
Copy link

PRs are welcome!

Isn't #1964 already open?

@mcollina
Copy link
Member

Actually it's abandoned with failing tests. A fresh one would be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Types Changes related to the TypeScript definitions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants