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 extensions #975

Closed
ronag opened this issue Aug 17, 2021 · 6 comments
Closed

Node extensions #975

ronag opened this issue Aug 17, 2021 · 6 comments
Labels
fetch question [Use a Discussion instead]

Comments

@ronag
Copy link
Member

ronag commented Aug 17, 2021

What extension to the spec (for a "non-strict" mode) do we believe makes sense for usage in node?

Some things I can think of:

@ronag ronag added question [Use a Discussion instead] fetch labels Aug 17, 2021
@ronag
Copy link
Member Author

ronag commented Aug 17, 2021

@Ethan-Arrowood @szmarczak

@szmarczak
Copy link
Member

Accept AsyncIterable (including stream.Readable) and Iterable as Request body.

👍

Accept flat raw headers array for Headers.

Why?

Accept EventEmitter as AbortSignal.

Why?

@ronag
Copy link
Member Author

ronag commented Aug 17, 2021

Accept flat raw headers array for Headers.

Why?

Performance

Accept EventEmitter as AbortSignal.

Why?

Not sure. A lot of node libs seems to do this. Maybe support for node versions without AbortSignal?

@szmarczak
Copy link
Member

Performance

Have you benchmarked this already? I don't think this will matter given the overhead...

Maybe support for node versions without AbortSignal?

Isn't fetch already Node.js 16+?

@ronag
Copy link
Member Author

ronag commented Aug 17, 2021

Performance

Have you benchmarked this already? I don't think this will matter given the overhead...

@Ethan-Arrowood Did some work on this. It's mostly for the cases where the headers are already in that format, e.g. node core rawHeaders or headers from undici core api. So I guess it's not just perf it's also compat.

Maybe support for node versions without AbortSignal?

Isn't fetch already Node.js 16+?

Yep. So maybe not relevant.

@szmarczak
Copy link
Member

It's mostly for the cases where the headers are already in that format

👍

ronag added a commit that referenced this issue Aug 17, 2021
ronag added a commit that referenced this issue Aug 17, 2021
@ronag ronag closed this as completed Aug 18, 2021
KhafraDev pushed a commit to KhafraDev/undici that referenced this issue Jun 23, 2022
crysmags pushed a commit to crysmags/undici that referenced this issue Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetch question [Use a Discussion instead]
Projects
None yet
Development

No branches or pull requests

2 participants