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

Improve the Fetch API #188

Merged
merged 8 commits into from
Mar 11, 2022
Merged

Improve the Fetch API #188

merged 8 commits into from
Mar 11, 2022

Conversation

richard-uk1
Copy link
Contributor

I needed a fetch interface, and I didn't find any of the existing options exactly fit my needs (I needed access to some of the web-only options), so I implemented it in gloo.

The implementation tries to be as unopinionated as possible, just doing things like strongly typing results, and discarding impossible errors. I'm PRing it in case you're interested in having it upstream, but I'm also happy to just keep a private fork if you don't want it.

///
/// If you are going to insert new headers, note that
/// [some names are forbidden](https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name)
/// and if they are set then sending the request will error.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to check the spec to see what happens, it's possible the offending headers are silently discarded.

@ranile
Copy link
Collaborator

ranile commented Feb 9, 2022

Have you seen reqwasm? If not, please have a look. It tries to be exactly what you're PRing and then some.

@richard-uk1
Copy link
Contributor Author

Have you seen reqwasm? If not, please have a look. It tries to be exactly what you're PRing and then some.

Cool! your crate mostly superseeds my work, but there are a couple of points where I've done more (e.g. doc comments from MDN). It would be good to get reqwasm into gloo (possibly as gloo::unstable::net to start with, to give it time to stabilize). Then people can start hacking on it and improving it.

ranile added a commit that referenced this pull request Feb 15, 2022
…oo-reqwasm

This merge is done to move reqwasm crate to gloo. This puts reqwasm
under the gloo umbrella.

See #188 (comment)
for need of this merge.
@ranile ranile mentioned this pull request Feb 15, 2022
@ranile
Copy link
Collaborator

ranile commented Feb 15, 2022

I have created #191

Once that is merged, I encourage you to update your branch to build off of it. One big improvement I can see is documentation: your docs are much better.

@richard-uk1
Copy link
Contributor Author

Yep happy to rebase off of your patch, which will make this PR mostly docs. :)

@ranile
Copy link
Collaborator

ranile commented Feb 16, 2022

I have merged the PR. You can rebase now

@ranile
Copy link
Collaborator

ranile commented Feb 25, 2022

@derekdreery any update on this? Are you working on the rebase?

@richard-uk1
Copy link
Contributor Author

Yes apologies I've been busy with work. Don't want to make promises but will look at ASAP.

# Conflicts:
#	crates/net/Cargo.toml
#	crates/net/README.md
#	crates/net/src/http.rs
@ranile
Copy link
Collaborator

ranile commented Mar 11, 2022

I went ahead and merged master into this branch

@ranile ranile changed the title Implement the Fetch API Improve the Fetch API Mar 11, 2022
@ranile ranile merged commit 07ad0fc into rustwasm:master Mar 11, 2022
@richard-uk1 richard-uk1 deleted the fetch2 branch March 11, 2022 10:43
@richard-uk1
Copy link
Contributor Author

Thanks for doing this - sorry I haven't had time.

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.

2 participants