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

Pass all Ky options to hooks #188

Merged
merged 5 commits into from
Nov 7, 2019

Conversation

sholladay
Copy link
Collaborator

@sholladay sholladay commented Oct 29, 2019

Closes #167
Closes #182
Closes #187

This PR:

  • Updates the options object passed to hooks so that all Ky and fetch options are included. For context: in Ky v0.15.0 and earlier, a subset of Ky options were included in the options object (e.g. prefixUrl was included, but throwHttpErrors was not, without any clear justification).
  • Loosens the validation of certain options so that hooks can pass their options (which are already normalized) to Ky to make a new request using the same options. For example, the json option now takes precedence over body, rather than being mutually exclusive with body. Additionally, searchParams can now be any value supported by URLSearchParams(), which makes it possible to use a boolean as a param value, provide the params as an array of entries, etc.
  • Removes our reliance on the document object, which was previously used in some circumstances to determine the base URL of the page, from which we resolve relative URLs. The Request class is now used to achieve the same goal. Request is significantly easier to polyfill and is much more likely to be available in web workers and non-browser environments.
  • Drops support for Sets throughout the API for simplicity. This should not have any meaningful performance impacts, since the lists of HTTP methods and status codes are inherently small in size, even if you were to include all valid methods and status codes. See Pass all Ky options to hooks #188 (comment).

index.d.ts Outdated Show resolved Hide resolved
@szmarczak szmarczak merged commit a5c249e into sindresorhus:master Nov 7, 2019
@sindresorhus
Copy link
Owner

@sholladay Thanks for doing this much needed improvement and cleanup. 🙌 Do you plan any other changes or should I go ahead and release this?

@sholladay
Copy link
Collaborator Author

@sindresorhus should be good to release. 👍

I will be working on #193 late tonight/tomorrow. Do you think we should consider that a breaking change (it would error previously)? If so, maybe we'd want to include it in this release. But I'm fine with letting it slip to the next one.

@sholladay sholladay deleted the better-options-handling branch November 7, 2019 21:50
@sindresorhus
Copy link
Owner

Almost anything is a breaking change for someone. I can wait to publish a new release. No rush.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants