Use KyRequest and KyResponse types in errors

sindresorhus / ky

🌳 Tiny & elegant JavaScript HTTP client based on the Fetch API

MIT License

13.46k stars 361 forks source link

Use KyRequest and KyResponse types in errors #610

Closed mfulton26 closed 1 month ago

mfulton26 commented 1 month ago

introduces KyRequest which, like KyResponse, adds json() convenience method for extracting JSON with an expected TypeScript type
update API Request/Response usages to expose KyRequest/KyResponse to ky consumers so that they may use the json() convenience method in more scenarios (HTTPError, TimeoutError, and hooks)

Closes #584

sholladay commented 1 month ago

LGTM, but another TS user should review, too.

Do we have a decent place to document this? Perhaps the readme should mention it.

mfulton26 commented 1 month ago

Do we have a decent place to document this? Perhaps the readme should mention it.

That's a good point. The existing KyResponse usage and stronger typed .json() isn't documented anywhere from what I can tell. It is discoverable via autocompletion (which is how I found it). I could see having a TypeScript and/or KyRequest/KyResponse section in the readme down with HTTPError and TimeoutError… they aren't Error subclasses, but they are top-level types.

sholladay commented 1 month ago

Yeah, I don't think the documentation about TS needs to be extensive. Autocomplete should be sufficient for most users. But I think it would be good to mention it somewhere, maybe as a feature in the benefits section at the top of the readme.

mfulton26 commented 1 month ago

@sholladay I took a stab at mentioning these TS improvements in the readme's benefits section. I tried to keep it short like the other bullet points. Suggestions welcome (more words, less, etc.).

sholladay commented 1 month ago

Good enough for me. We can tweak it later if needed. I'm going to merge this now since we have quite a few open PRs and I want to avoid creating unnecessary conflicts by leaving things open.

Thanks again!