search

nodejs / undici

An HTTP/1.1 client, written from scratch for Node.js
https://nodejs.github.io/undici
MIT License
6.18k stars 537 forks source link

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

Open kettanaito opened 1 year ago

kettanaito commented 1 year ago

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:

https://github.com/nodejs/undici/blob/d8d9a96b6b8f856d9e01a1edfe22c3d83c6996ec/types/fetch.d.ts#L138

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

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 (https://github.com/mswjs/interceptors/pull/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.

KhafraDev commented 1 year ago

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

kettanaito commented 1 year ago

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

kettanaito commented 1 year ago

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 commented 1 year ago

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

mcollina commented 12 months ago

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

PRs are welcome!

NatoBoram commented 10 months ago

PRs are welcome!

Isn't https://github.com/nodejs/undici/pull/1964 already open?

mcollina commented 10 months ago

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