Query strings should be modeled as a multimap

[queensferryme]

As indicated by the Wikipedia page Query string:

While there is no definitive standard, most web frameworks allow multiple values to be associated with a single field (e.g. field1=value1&field1=value2&field2=value3).

And indeed, many web frameworks I have used adopt this approach:

• Django: https://docs.djangoproject.com/en/3.2/ref/request-response/#django.http.QueryDict.getlist
• Starlette: https://www.starlette.io/requests/#query-parameters
• nodejs/querystring: https://nodejs.org/api/querystring.html#querystringparsestr-sep-eq-options

Therefore, I suggest itty-router follow the same convention.

Currently, itty-router models query strings as a Obj:

https://github.com/kwhitley/itty-router/blob/e8e76c9be5496ce2494743fb20dff802c41eb176/src/itty-router.d.ts#L22

where Obj is actually a map from some string to some string:

https://github.com/kwhitley/itty-router/blob/e8e76c9be5496ce2494743fb20dff802c41eb176/src/itty-router.d.ts#L1-L3

I suggest introducing MultiObj as the data model of query strings:

export type MultiObj = {
    [propName: string]: string | Array<string>
}

I'd like to help if you @kwhitley are interested in this :)

[oliverjam]

Would it not be better to stick to web standard APIs, since CF Workers/itty already rely on Request, Response etc? The URLSearchParams interface is specifically designed for this:

let query = new URLSearchParams("field1=value1&field1=value2&field2=value3");

query.get("field2"); // "value3"
query.get("field1") // "value1" (picks the first)
query.getAll("field1"); // ["value1", "value2"] (array of all matches)
query.getAll("field2"); // ["value3"] (always an array for type consistency)

for (let [name, value] of query) {
  console.log(name, value);
}
// field1 value1
// field1 value2
// field2 value3

[kwhitley]

Well folks great point you've brought up... but a couple issues with this...

1. The option @queensferryme suggests would be non-standard, but technically compatible with the existing API, thus probably not require a major version bump. The downside is, it would also add a chunk of code to the code-golfed itty to support this. In this case, I'd recommend middleware to do this for you (certainly for now anyway), as you can still access the request.url yourself and write over the convenience object request.query that itty creates for you.

2. I do agree with @oliverjam re. sticking to web standards as much as possible, although I didn't do that with the original itty as I was trying to more closely mirror some of the nice/light express API. The problem with switching to this approach, while adding essentially no characters (nice!), is that it definitely is a breaking change.

3. I think an easier short term answer is to add an upstream middleware to itty-router-extras that tackles one of the above approaches, with could be implemented like this in your code:

   
   import { Router } from itty-router
   import { withQuery } from 'itty-router-extras'

const router = Router()

router .all('*', withQuery) .get('/foo', ({ query }) => { // query now has altered shape })

[oliverjam]

I'd be happy with the opt-in middleware approach.

Alternatively you could leave query as is (and maybe deprecate for removal in a future major release), then add a search or searchParams property with the new searchParams interface. That's what it's called for the web standard URL object (new URL("blah.com?test=5").searchParams). I guess that would add precious bytes though 😅

[spencerwooo]

So...where are we on this?

[bever1337]

I believe accepting PR 105 -- https://github.com/kwhitley/itty-router/pull/105 -- is the correct approach to giving developers access to the spec-compliant searchParams.

I don't see the risk in continuing to mutate the request to have the itty-specific params property. If developers want to disregard the non-compliant params, they can do so. Developers can also use JSDoc or Typescript to specify the request type as a standard Request parameter to their handler functions, easy peasey!

I think PR #105 is a feature bump, not a major bump, if it only adds additional properties to the typings.

From a 'correctness' standpoint, I have an itch to see the params removed in a major version, but I think it makes more sense to treat it as legacy. There are other non-standard properties that Itty adds, this seems appropriate in that context.

These are just my two cents, of course. Thanks!

[kwhitley]

I believe accepting PR 105 -- #105 -- is the correct approach to giving developers access to the spec-compliant searchParams.

I see the allure, but nothing about itty is spec-compliant. It (intentionally) doesn't require a real Request, and nor does it just spit you back out a native Request. It embeds helpers (e.g. params, query) with a single purpose in mind:

1. Make downstream code lighter.

This does, of course, mean that it won't always follow spec or even traditional paths. It's great when it can, but if spec = verbose, I change the internal path. In the end, I want route handlers to be as lightweight and human-readable as possible, even if that means breaking some conventions.

Now, to the original point of this issue, I see the point, and I think I can have "query" behave in the intended way. That was more of an oversight from myself than an intended effect, and as it would only "fix" people's code that had previously been affected by said pattern, I wouldn't classify it as a major bump - as in, no one should currently be relying on the unexpected way of it discarding multiple values on the same param, because that would really just not make sense.

Yes it'll add some bytes, but it's about to grow a tiny bit anyway as I wrap the itty-router-extras into the core lib (all for the sake of a single, simple solution to "tiny code"). Definitely tired of seeing the raw Response building code in the itty docs, as well as tutorials/blogs around the web. Itty was never intended to be used as a standalone, but was released as such to achieve the code-golfing-esque ~400bytes (at the time). In the end, that was a useless vanity metric, and it's time I made it more generically useful to serve the broader audience.

From a 'correctness' standpoint, I have an itch to see the params removed in a major version, but I think it makes more sense to treat it as legacy. There are other non-standard properties that Itty adds, this seems appropriate in that context.

request.params is a familiarity thing... and is in not only Express, but virtually all Express-inspired routers (itty included)... it references route params, not searchParams.

[kwhitley]

This all said, I do love the discussion and the feedback - keep it coming! :)

[bever1337]

... it references route params, not searchParams.

Slip of the keyboard there, thanks for correctly reading into what I meant to say! Awesome feedback, this is helpful as an 'itty manifesto.' For example, I didn't know itty intentionally avoided Request objects.

[kwhitley]

This will be included in the next major release :)

[kwhitley]

Closing this, as #131 addresses it :)