Add URI.encode_query/2

80 views
Skip to first unread message

spencer...@gmail.com

unread,
Nov 5, 2020, 1:27:36 PM11/5/20
to elixir-lang-core
I would like to propose the addition of URI.encode_query/2 to the URI module to facilitate URI construction in a pipe friendly fashion.

Here is an example MR on my fork of elixir
https://github.com/spencerdcarlson/elixir/pull/2

Andrea Leopardi

unread,
Nov 6, 2020, 3:16:59 AM11/6/20
to elixir-lang-core
Hey Spencer,

Considering how concise the snippet of code you showed is even without URI.encode_query/2, I think the value of having a smaller API surface is higher than the value of adding a function that you can replace with just a few more characters. Thoughts?

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/7e297574-b4d0-4b03-8279-4abe50c191e0n%40googlegroups.com.

Bruce Tate

unread,
Nov 6, 2020, 7:18:16 AM11/6/20
to elixir-l...@googlegroups.com
I think the pull request is a much better API. It both takes and returns a URI, and also gains type checking benefits (and readability benefits... single level of abstraction). 

-bt



--

Regards,
Bruce Tate
CEO

José Valim

unread,
Nov 6, 2020, 7:49:32 AM11/6/20
to elixir-l...@googlegroups.com
Hi Spencer, thanks for the proposal.

My main question is what happens if the URI already has a query. Do we update it? Do we replace it? On this question alone, if we were to add such functionality, then I think it should not be called encode_query but rather update_query/replace_query.

update_query itself has ambiguous connotations as well, in that it can either update_query (replacing duplicate keys) or simply append to it since a query can have duplicate keys (so maybe it should be called append_query?). Note how a web server will handle duplicate keys is completely up to the web server implementation, so having an implementation that appends by default can lead to surprising behaviour.

The implementation of the PR follows the append_query semantics - but we need to discuss all of those things if we are to accept such a proposal because these semantics need to make sense in the general case.

The other thing to note is that "URI.encode_query" is one of many ways of encoding a query string. Web applications have historically built more complex query encoding/decoding schemas that (poorly) support richer data structure, so this API may not be enough depending on what you want to encode.



spencer...@gmail.com

unread,
Nov 12, 2020, 9:51:55 PM11/12/20
to elixir-lang-core
Thanks for the thoughtful feedback. 

I think we should create URI.append_query/1 and specify in documentation that duplicate keys are not considered and that it uses URI.encode_query for encoding. I believe that "append" as used in Tupple.append/2 is clear enough to indicate key duplication without having to read the documentation.

If we want to address updating query params we could also create URI.put_query/3 and URI.put_new_query/3 which would follow the same logic and naming implications of both the Map and Keyword module functions where "put" adds and updates if it exists, while "put_new" only adds if it does not exist.

You raise a good point about the possible limitations of URI.encode_query/1 but I think that should be solved as a separate issue. URI.encode_query/1 is currently the only option in the URI module, so I don't see any harm in using it internally in another URI function. Maybe one solution could be to augment URI.encode_query/1 with options that could also be passed into the "put" and "put_new" functions. Overall, I think that the complexity of URI.encode_query/1 could be address in another request for change.

There is a significant value add in at least adding URI.append_query/1 to the API to allow developers to stay within the URI abstraction when building a URI and to push the URI module in the direction of a subject as first argument module.


Reply all
Reply to author
Forward
0 new messages