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

gloo-net: Allow RequestBuilder.query to accept a struct that implements serde::Serialize as an argument #378

Open
SleeplessOne1917 opened this issue Oct 2, 2023 · 1 comment

Comments

@SleeplessOne1917
Copy link

Summary

As the title says, allow the query method on RequestBuilder to accept a type that implements serde::Serialize.

Motivation

The current signature for query works well when you know upfront what you want the keys for your params to be, but is unwieldy if you have to convert different objects with arbitrary property names into query params. Looking at 2 other popular http client libraries, reqwest and awc, they allow the passing of objects that implement Serialize.

Detailed Explanation

I'm not familiar with rust yet to have a solid plan for implementation, but the usage of the API for clients would look something like this:

#[derive(Clone, Serialize, Deserialize)]
struct SendMe {
    pub foo: String,
    pub bar: String
}

async fn do_request(args: SendMe) {
    Request::get("http://website.com/api")
                    .query(args) // No need to convert this into a map or vec of tuples
                    .send().await;
}

Drawbacks, Rationale, and Alternatives

The biggest drawback I can predict is trying to find a way to implement this without breaking the existing API.

As for alternatives, if there are any easy ways to convert a struct to a hashmap this change might be unnecessary. The way I currently found to get around it was to use serde_urlencoded to convert a serialize object into a query string, then split the string into several key value pairs, and finally split the key value pairs into their keys and values. This is annoying enough that it inspired me to make this issue.

Unresolved Questions

n/a (I can't think of any, but maybe reviewers can)

@SleeplessOne1917 SleeplessOne1917 changed the title gloo-net: Make RequestBuilder.query to accept a struct the implements serde::Serialize as an argument gloo-net: Allow RequestBuilder.query to accept a struct the implements serde::Serialize as an argument Oct 2, 2023
@SleeplessOne1917 SleeplessOne1917 changed the title gloo-net: Allow RequestBuilder.query to accept a struct the implements serde::Serialize as an argument gloo-net: Allow RequestBuilder.query to accept a struct that implements serde::Serialize as an argument Oct 2, 2023
@hamza1311
Copy link
Collaborator

hamza1311 commented Oct 4, 2023

The QueryPararms don't use serde, they use the browser's implemention: URLSearchParams.

This change isn't possible without making that change. Also see: #215

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

No branches or pull requests

2 participants