Skip to content

Instantly share code, notes, and snippets.

@iStefo
Created February 28, 2014 08:54
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save iStefo/9267640 to your computer and use it in GitHub Desktop.
Save iStefo/9267640 to your computer and use it in GitHub Desktop.
my thoughts on query-params-new

These are my thoughts on using the (bleeding edge) query-params-new from 1.6.0-beta.1+canary.01f3f32c.

1. Numerical Parameters

The currently handled parameter types are strings (of course), arrays (with the arr[]=foo1&arr[]=foo2 syntax, which is not the nicest but closest zu the standard) and booleans. The absence of integer parameters means having to use parseInt() in every method where the parameter is used. (Setting the parameter to a number value is no problem)

I think there are two ways to determine whether to cast or not while parsing parameters:

  1. Check if param contains only digits and just assume that it's meant to be a numeric parameter then. Since this may cause unforeseen behaviour we shouldn't do this
  2. Add a descriptior to the controllers queryParams to force a query parameters data type (this would also allow providing an array when the present url is arr=foo1 without the [], but I'm not sure if this is a desirable thing)

2. Default values

This is a big one for my project. The simplest example is the pagination which starts at page=1, which because its the home page should not need a set query param. Of course, I could start with page=false to hide the parameter from the url but then I loose the ability to just bind this value to the pagination controller and have unecessary if-clouses to handle this case.

Also, when using string values as query params and the value gets set to "" (empty string) because the user clears the search field, the query is still search=, which is not nice. I wrote an observer method clearing empty query parameters, but maybe there should exist a common way to declare an empty parameter as not needed in the url:

clearStringQueryParams: function(target, changedKey) {
    if (target.get(changedKey + '.length') === 0) {
        target.set(changedKey, null);
    }
}.observes('search', 'channel', 'author', ...),

Again, this could be achieved by a queryParam descriptor instead of the simple string in the queryParams array.

I think these changes would really improve the experience when working with query params. Special thanks especially to @machty for his work on this topic.

Best, Stefan

@machty
Copy link

machty commented Feb 28, 2014

These are great points, and I think they'll be addressed by the following:

  • Instead of having a starting value for a controller prop, we have a default value which is the value of that property if no such overriding value is specified in the URL
  • We can infer proper (de)serialization based on the type of the default value (no more parseInt all over the place)
  • If this still leaves too many holes we can open things up to allow for custom (de)serializers

Any concerns with this approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment