search

microsoft / TypeScript-DOM-lib-generator

Tool for generating dom related TypeScript and JavaScript library files
Apache License 2.0
600 stars 417 forks source link

Update Body json() method to allow generic types. #1711

Closed incogiscool closed 2 months ago

incogiscool commented 2 months ago

This all started when I tried to use the Body > json() API for my server using Typescript.

Unfortunately, the json() function returns a Promise<any> type, which is not ideal when trying to build type-safe code. A way I got around this was by using the as operator, which allowed me to cast my types. This isn't a completely ideal method but it worked.

const data = (await request.json()) as SetCreatorDataRequest;

A much better way to use this API with type-safety would be to how axios does it:

axios.get<MyTypeHere>(...)

So I adjusted the Body > json() function so that it allows for a generic type (T), and defaults to an any type.

json<T = any>(): Promise<T>

Hopefully this pull request gets implemented and becomes a feature of the Typescript DOM API.

Thanks :)

github-actions[bot] commented 2 months ago

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

saschanaz commented 2 months ago

I'm pretty sure there was a similar PR years ago which was rejected because "we only add type parameter when it's actually used in paremeters, for this case you can just cast the return type".

Is that changed, i.e. are there similar declarations as this PR does in existing JS types in TypeScript?

incogiscool commented 2 months ago

I'm pretty sure there was a similar PR years ago which was rejected because "we only add type parameter when it's actually used in paremeters, for this case you can just cast the return type".

Is that changed, i.e. are there similar declarations as this PR does in existing JS types in TypeScript?

Ah, I wasn't aware of the existing PR. I'm not sure if there are similar declarations - but wouldn't adding a generic allow for cleaner and more supported code down the line over just casting a variable?

Also, if you can find it, I would love to review the old PR and understand why it was rejected.

Thanks!

incogiscool commented 2 months ago

Hey, I was just wondering if there is any update on the state of this PR. Is there anything I need to change, or should I close if It won't get implemented?

Thanks.

saschanaz commented 2 months ago

This was my previous response, which still seems valid: https://github.com/microsoft/TypeScript-DOM-lib-generator/pull/1078#issuecomment-889181856