Description
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 having Header be a wrapper over web_sys::Header we could choose to wrap a http::HeaderMap and implement From<Header> for web_sys::Header. In that future, we'd be happier to have committed to
Header::set(&mut self, key: &str, value: &str)
Over
Header::set(&self, key: &str, value: &str)
These changes can also open the door to improve https://rust-lang.github.io/api-guidelines/interoperability.html#interoperability.
Detailed Explanation
Request and Response
In #312 we added RequestBuilder and ResponseBuilder 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.
// mutable
pub struct RequestBuilder(...);
impl RequestBuilder {
pub fn from_raw(raw: websys::RequestInit) -> Self;
pub fn into_raw(self) -> websys::RequestInit;
// Creates a new request with the provided method
pub fn get(url: &str) -> RequestBuilder;
pub fn post(url: &str) -> RequestBuilder;
// ...
// Setters
pub fn header(self, header: http::HeaderName, value: http::HeaderValue) -> Self; // or strings
pub fn headers(self, headers: http::HeadersMap) -> Self;
// ...
// Getters
// I'm still investigating if these builders are infailable. They might not be.
pub fn body(self, body: Body) -> Request;
pub fn json<T: serde::Serialize>(self, value: &T) -> Request;
pub fn binary(self, body: Vec<u8>) -> Request;
pub fn build() -> Request
}ResponseBuilder has a similar API.
Response and Request map into web_sys::Response and web_sys::Request.
// immutable
pub struct Request(...);
impl Request {
pub fn from_raw(raw: web_sys::Request) -> Self;
pub fn into_raw(self) -> web_sys::Request;
// To construct a Request should make a builder
pub fn get(url: &str) -> RequestBuilder;
pub fn post(url: &str) -> RequestBuilder;
// ...
// I'm propsing changes to these getters to return _borrowed_ values
// In my opinion a Request should be immutable after creation, like other libraries
pub fn headers(&self) -> &Headers;
pub fn method(&self) -> &http::Method;
pub fn url(&self) -> &String;
// ...
// You can extract the body of a request/response, only once
pub async fn body(self) -> Body;
pub async fn text(self) -> Result<String, Error>;
pub async fn binary(self) -> Result<Vec<u8>, Error>;
// `fetch()`
pub async fn send(self) -> Result<Response, Error>;
}
impl Clone for Request {
// This is https://developer.mozilla.org/en-US/docs/Web/API/Request/clone
// It works by `tee`-ing the body of the request, in JS clone misbehaves if
// the body has already been consumed. Since you can only extract the body
// once, `clone` should always work
}Headers
Remove Headers and replace it with http::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 fixes
When wrapping a web_sys type, we exclusively get owned values back, for example
// web_sys::Request
#[doc = "Getter for the `method` field of this object."]
#[doc = ""]
#[doc = "[MDN Documentation](https://developer.mozilla.org/en-US/docs/Web/API/Request/method)"]
#[doc = ""]
#[doc = "*This API requires the following crate features to be activated: `Request`*"]
pub fn method(this: &Request) -> String; // Returns an owned value
// Our wrapper
pub fn method(&self) -> &http::Method; // Returns a borrowed valueI'm proposing adding a new Rust type RequestOptions that stores rust representations of values we used to keep in a web_sys::RequestInit.
Like so:
#[derive(Debug)]
struct RequestOptions {
method: http::Method,
headers: http::HeaderMap, // or our current `Headers` type
body: Option<Body>,
cache: web_sys::RequestCache,
credentials: web_sys::RequestCredentials,
integrity: String,
mode: web_sys::RequestMode,
redirect: web_sys::RequestRedirect,
referrer: String,
referrer_policy: web_sys::ReferrerPolicy,
// signal: Option<&'a web_sys::AbortSignal>, TODO: our requests could be made cancellable
}
impl From<RequestOptions> for web_sys::RequestInit {...} // straightforward
impl From<web_sys::RequestInit> for RequestOptions {...} // makes significant use of reflection.
// Which makes the builder
pub struct RequestBuilder {
// url
url: String,
init: RequestOptions,
}
// And to support the getters in the Request
pub struct Request {
inner: web_sys::Request,
// Cached Rust representations of the request inners.
url: String,
init: RequestOptions,
}Unresolved Questions
We can maybe remove Request::inner and instead lazily create it when well call send or try to process the body.
TODO