gleam-wisp / wisp

🧚 A practical web framework for Gleam
https://gleam-wisp.github.io/wisp/
Apache License 2.0
915 stars 41 forks source link

A better way to access query string params #39

Closed thelinuxlich closed 1 year ago

thelinuxlich commented 1 year ago

Right now we can do it like this:

// param "foo" should not be empty
use params <- try(get_query(req))
use #(_, search_term) <- try(list.find(
  params,
  fn(x) { x.0 == "foo" && x.1 != "" },
))

Ideally we'd have something to enforce the existence of query params like:

use params <- wisp.require_query_params(req, list_of_param_names) // params : Map(name, value)

// when you just want the whole thing without enforcing anything
use params <- wisp.qs_to_map(req) // params: Map(name, value)
lpil commented 1 year ago

It gets a little trickier when you consider that query parameters can't be stored in a map- the ordering matters and keys can be duplicated.

If nothing else a get_query(r: Request) -> List(#(String, String)) would be an improvement, but perhaps we can do better.

thelinuxlich commented 1 year ago

Doesn't frameworks take a stance about how to parse duplicate query parameters? Like some parse "key[]=foo" into key: ["foo"] ?

lpil commented 1 year ago

I've been exploring API designs this evening and I'm not making much progress.

use params <- wisp.require_query_params(req, list_of_param_names) // params : Map(name, value)

Why is it that a list of parameter names is passed in here? You'd still need to map.get on the params, so each parameter name would be in the code twice.

use #(_, search_term) <- try(list.find(
  params,
  fn(x) { x.0 == "foo" && x.1 != "" },
))

Here there's an extra requirement of ensuring the query parameter doesn't have an empty value, which we also wouldn't want. Empty values are often useful.

thelinuxlich commented 1 year ago

The first was just an example of how I use it, not to be added to wisp. Maybe we can reuse https://github.com/sporto/gleam_qs ?

lpil commented 1 year ago

If we used that custom type based API the programmer would need to add a case for each parameter, which seems annoying than using list.key_find.

thelinuxlich commented 1 year ago

so maybe just a wisp.get_query_as_list(req) ?

lpil commented 1 year ago

Which would be a re-export of request.get_query?

thelinuxlich commented 1 year ago

No, should be a method that actually parses the qs

lpil commented 1 year ago

That's what request.get_query does, aye

pub fn get_query(request: Request(a)) -> Result(List(#(String, String)), Nil)
thelinuxlich commented 1 year ago

yeah, I was barely awake when I wrote that, what I mean is we need a method that parses the qs and take a stance on arrays

lpil commented 1 year ago

It does- it keeps them. You can filter the list to get all the values, which is more concise an API than the custom type based approach in that library which forces you to use a case expression every time.