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

FormDataLike iterator type is incorrect #19

Closed
Ethan-Arrowood opened this issue Oct 23, 2023 · 4 comments
Closed

FormDataLike iterator type is incorrect #19

Ethan-Arrowood opened this issue Oct 23, 2023 · 4 comments
Labels
bug Something isn't working TypeScript TypeScript-related changes, like types or build its build infrastructure

Comments

@Ethan-Arrowood
Copy link

The iterator type for FormDataLike is incorrect / too restrictive (

[Symbol.iterator](): Generator<[string, FormDataEntryValue]>
). It uses Generator which makes methods throw and return required. But based on the spec, those two are optional for iterators (see here in MDN docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#throwexception) which are linked to in the source of FormDataLike itself.

I believe the types in undici are more accurate (https://github.com/nodejs/undici/blob/8050ec0224a51d44f776364820e6a16112fb4781/types/formdata.d.ts#L105 and https://github.com/nodejs/undici/blob/8050ec0224a51d44f776364820e6a16112fb4781/types/fetch.d.ts#L41-L47).

This is actually how I found this issue. I was trying to use the FormData object from Undici with the FormDataEncoder from this library.

@octet-stream
Copy link
Owner

Hi, thanks for your feedback. Yeah, this makes sense. TypeScript itself seem to use IterableIterator<T> for this method (see: https://github.com/microsoft/TypeScript/blob/d9d027d0c4f5044631a0c078166b8d01f41d28b5/src/lib/dom.iterable.generated.d.ts#L98). I was a little confused by the differences between Generator<T, TReturn, TNext>, Iterator<T> and IterableIterator<T> types. Though, I have no idea why undici needs their own typing for this.

@octet-stream octet-stream added bug Something isn't working TypeScript TypeScript-related changes, like types or build its build infrastructure labels Oct 23, 2023
@Ethan-Arrowood
Copy link
Author

That's a good point. Not sure either. But I think switching to IterableIterator<T> will be good change nonetheless.

@octet-stream
Copy link
Owner

octet-stream commented Oct 23, 2023

@Ethan-Arrowood
Copy link
Author

Incredible, thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working TypeScript TypeScript-related changes, like types or build its build infrastructure
Projects
None yet
Development

No branches or pull requests

2 participants