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

Add Fetch support directly into the fork #5

Closed
TheSpyder opened this issue Mar 24, 2021 · 6 comments · Fixed by #31
Closed

Add Fetch support directly into the fork #5

TheSpyder opened this issue Mar 24, 2021 · 6 comments · Fixed by #31
Labels
enhancement New feature or request

Comments

@TheSpyder
Copy link
Owner

This library currently depends on https://github.com/reasonml-community/bs-fetch but unlike JS packages ReScript dependencies aren't automatic. In order to compile with ReScript bs-fetch must be added to the top level bsconfig.json of projects that want to start using rescript-webapi, even if the parts that depend on bs-fetch are never used. This is an annoyance I'd rather avoid.

The licenses are compatible, we can just pull it in and apply similar improvements to what we're planning here (looks like there are some records-as-objects opportunities).

@spocke spocke added the enhancement New feature or request label Jun 3, 2021
@TheSpyder
Copy link
Owner Author

When we do this, add AbortController support at the same time:
https://developers.google.com/web/updates/2017/09/abortable-fetch

Abortable fetch is available in all modern browsers.

@TheSpyder
Copy link
Owner Author

This is now very high priority - ReScript 10 is dropping bs.send.pipe. That's not due out until January, but we still need to be ready (even if 1.0 isn't done, we at least need a release with pipe-first fetch support).

@TheSpyder
Copy link
Owner Author

When we do this, add AbortController support at the same time:
https://developers.google.com/web/updates/2017/09/abortable-fetch

Abortable fetch is available in all modern browsers.

Apparently I just wasn't paying attention...
https://github.com/reasonml-community/bs-fetch/blob/934389964e1005d4e37911c8324a6aeb7ce0a1b0/src/Fetch.mli#L35-L45

Although to be fair while the type is defined it's not available for use 🤔

@TheSpyder
Copy link
Owner Author

@TheSpyder
Copy link
Owner Author

As much as I'd like to update the API, there are over 1300 repositories depending on bs-fetch. Now that I know ReScript 10 will break their code, I want to start by making migration as seamless as possible for them.

I do have plenty of ideas for ways the API can be improved - logged #30 to work on them. If I have time for both the API update and a migration guide before ReScript 10 is released we can consider merging it.

@TheSpyder
Copy link
Owner Author

Released in 0.1.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants