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

fix(readBody): set default return type to unknown #387

Closed
wants to merge 1 commit into from

Conversation

rijkvanzanten
Copy link

Fixes #386

@rijkvanzanten rijkvanzanten changed the title Set default return type of readBody to unknown fix: set default return type of readBody to unknown May 16, 2023
@pi0 pi0 changed the title fix: set default return type of readBody to unknown fix(readBody): set default return type to unknown Jun 9, 2023
@pi0
Copy link
Member

pi0 commented Jun 9, 2023

The return type of readBody is consistent with JSON.parse and destr types which are both any as well and used in readBody. It is ultimately your responsibility to make sure body is both type validated and runtimeValidated. However i agree we need better type safety..

This change is also breaking for types in existing projects. I would be happy to discuss about possible breaking change in next major versions of h3/nitro or alternatively have a more specific util like readValidatedBody with more strict types. Followup: #402

Copy link
Member

I don't believe replacing with unknown is a breaking change; I would consider it fixing a bug with type safety.

Users who do not care about type safety in this kind of way can always set strict: false in order for TS to treat the unknown as any.

@rijkvanzanten
Copy link
Author

rijkvanzanten commented Jun 9, 2023

The return type of readBody is consistent with JSON.parse [...]

I'm pretty sure JSON.parse is any because it was typed before unknown even existed. There's also a long running suggestion around replacing those anys with unknowns for the same reason. (for example microsoft/TypeScript#27265)

[...] and destr types [...]

Which is your design decision! 😄 IMO that should be unknown as well

It is ultimately your responsibility to make sure body is both type validated and runtimeValidated. However i agree we need better type safety..

I fully agree, which is why I proposed this change. Right now, by default, h3 effectively disables type safety by setting it to any instead of making it my responsibility with an unknown.

@pi0
Copy link
Member

pi0 commented Jun 12, 2023

destr v2 is out with default unknown and generic type support. The next major version of h3 is planned soon alongside with object syntax for handlers so rest assured this change will happen soon and thanks for the idea.

@rijkvanzanten
Copy link
Author

@pi0 Is there any public issue tracker or milestone to keep up with the development of the next major version? 🙂

@pi0
Copy link
Member

pi0 commented Jul 27, 2023

We have added todos in the code to change from any to unknown in next future versions. Also any new utils that are being introduced will prefer unknown by default. (on a related note, other than JSON.parse, i noticed even response.json() with web standards used any by default -- nothing that stops us from this change!)

#459 adds safe and validated (type + runtime) alternative utils and #417 a much better way to type events input via the Event handler itself.

Let's keep tracking it by #386 (closing this PR to clear up open PRs) and thanks again for pushing forward this important change in typings.

@pi0 pi0 closed this Jul 27, 2023
@rijkvanzanten rijkvanzanten deleted the unknown-return-type branch July 27, 2023 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

readBody has a default returnType of Promise<any> instead of Promise<unknown>
5 participants