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

GET request URL string with multiple values for one parameter #113

Closed
a1exsh opened this issue Mar 27, 2018 · 5 comments
Closed

GET request URL string with multiple values for one parameter #113

a1exsh opened this issue Mar 27, 2018 · 5 comments

Comments

@a1exsh
Copy link
Contributor

a1exsh commented Mar 27, 2018

I've seen the discussion in #47, but my case is subtly different.

I need to call an API with a URL which contains multiple values for the same parameter (a list parameter), but I'm not building the URL: I'm receiving it as part of response from the API. So I can't just use the :query-params option unless I parse the URL myself before making the request. And I can't use parse-url because it ignores the multiple values:

=> (:query-params (cljs-http.client/parse-url "http://example.com/?a=1&b=2&a=3"))
{:a "3", :b "2"}

Cannot we really avoid parsing and building the URL in the cljs-http.client?

@r0man
Copy link
Owner

r0man commented Mar 28, 2018 via email

@a1exsh
Copy link
Contributor Author

a1exsh commented Mar 28, 2018

Yeah, I now see it is bit more complex.

Not parsing and re-building the URL will solve the case when you only have the :url and no additional params. But since the client already supports mixing :url and :query-params in a request, we would still have to parse it occasionally.

I wonder if we should rather make the parsing and building more flexible in order to be able to accommodate list parameters? E.g. adding an option to parse-query-params to collect multiple params in a list value, but keep the default behavior as it is now. At the same time generate-query-string should learn to handle list values.

@a1exsh
Copy link
Contributor Author

a1exsh commented Mar 31, 2018

Wait, generate-query-string is already aware of list handling, so this part is a no-brainer. ;)

@a1exsh
Copy link
Contributor Author

a1exsh commented Mar 31, 2018

After some consideration I believe that we should have the behavior of parsing into list by default, w/o an additional parameter to control this. This ensures that the URL is preserved in the process of parsing and rebuilding. If someone wants a finer control, the URL can still be parsed and adjusted to taste.

Opened a PR: #116

@a1exsh
Copy link
Contributor Author

a1exsh commented Apr 3, 2018

OK, let's close this now since #116 is merged. :)

@a1exsh a1exsh closed this as completed Apr 3, 2018
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