Open Alextopher opened 1 year ago
How is pub fn binary(self, body: Vec<u8>) -> Request;
accomplished in the current version?
It's been a long time since I've thought about this issue. If improvements have been added since I published this issue then it might be worth closing ❤️
:warning: This PR is still a bit of a draft, sorry about that, I'm about to start a new job and need to relearn C++ :smile: But hopefully there is enough here to start a discussion.
Summary
Make
gloo_net
more idiomatic by increasing ownership and borrowing constraints. As is the Rust way, this will prevent certain classes of bugs and make our implementations flexible for future improvements.This will require increasing the complexity of wrapper types.
Motivation
In JavaScript the bodies of requests and responses can only be read once, if you read the same response body more than once you'll just receive an empty buffer, without an error. This behavior is almost always a bug, and rust ownership semantics can prevent this. By requiring certain methods to take ownership of the underlying Request/Response, this bug is prevented.
JsValue
backed types allow interior mutability through shared references. If we do the same with our wrapper types (Request
,Response
,RequestBuilder
, etc.) we're adding unnecessary constraints to future implementations, and our APIs don't communicate mutability where it exists. One example is rather than havingHeader
be a wrapper overweb_sys::Header
we could choose to wrap ahttp::HeaderMap
and implementFrom<Header> for web_sys::Header
. In that future, we'd be happier to have committed toOver
These changes can also open the door to improve https://rust-lang.github.io/api-guidelines/interoperability.html#interoperability.
Detailed Explanation
Request
andResponse
In #312 we added
RequestBuilder
andResponseBuilder
those types map to RequestInit and ResponseInit. I don't believe that made it into an RFC, I would like to suggest minor changes to those new APIs.ResponseBuilder
has a similar API.Response
andRequest
map intoweb_sys::Response
andweb_sys::Request
.Headers
Remove
Headers
and replace it withhttp::HeadersMap
.TODO: explain why
Drawbacks, Rationale, and Alternatives
This RFC includes API breaking changes to basically every type in
gloo_net
. Since most changes involve ownership and borrowing,cargo check
should communicate most of the fixesWhen wrapping a
web_sys
type, we exclusively get owned values back, for exampleI'm proposing adding a new Rust type
RequestOptions
that stores rust representations of values we used to keep in aweb_sys::RequestInit
.Like so:
Unresolved Questions
We can maybe remove
Request::inner
and instead lazily create it when well callsend
or try to process the body.TODO