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

Use KyRequest and KyResponse types in errors #610

Conversation

mfulton26
Copy link
Contributor

@mfulton26 mfulton26 commented Jul 16, 2024

  • 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

- 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)

Refs: sindresorhus#584
@sholladay
Copy link
Collaborator

sholladay commented Jul 17, 2024

LGTM, but another TS user should review, too.

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

@mfulton26
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

@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
Copy link
Collaborator

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!

@sholladay sholladay changed the title feat: additional json convenience methods Use KyRequest and KyResponse types in errors Jul 19, 2024
@sholladay sholladay merged commit 8e171f5 into sindresorhus:main Jul 19, 2024
1 check passed
@mfulton26 mfulton26 deleted the feat--additional-json-convenience-methods branch July 19, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTPError json as unknown/type
2 participants