backbone icon indicating copy to clipboard operation
backbone copied to clipboard

Route parameters and ES6 default parameters

Open petetnt opened this issue 9 years ago • 4 comments

This is an rather small issue with route parameters and ES6 default parameters that I just ran into and thought I would just bring it up (semi-related to #3828 I guess):

Say we have an Router that looks like this:

const Router = Backbone.Router.extend({
    routes: {
        "(:foo)(/)(:bar)": "biz"       
    }
});

let router = new Router();

router.on("route:init", (foo, bar) => {
  console(foo, bar);
});

In backbone.js#L1532 the comment above the _extractParameters method states:

// Given a route, and a URL fragment that it matches, return the array of // extracted decoded parameters. Empty or unmatched parameters will be // treated as null to normalize cross-browser behavior.

When navigating to foo.com, we get the following output:

console.log(foo, bar) // null, null

There's nothing wrong with this, but it gets a bit more trickier when default parameters come in:

Say we want to use some default parameters if none are supplied, in ES5 we would do something like this:

router.on("route:init", (foo, bar) => {
  foo ? foo : "hamburger";
  bar ? bar : "vegan";
  console(foo, bar);
});

In ES6, one could just use default parameters:

router.on("route:init", (foo = "hamburger", bar = "vegan") => {
   console(foo, bar); // expected: hamburger, vegan
});

However, as MDN states:

Default function parameters allow formal parameters to be initialized with default values if no value or undefined is passed.

The output from that is still null, null because the passed in values are null, rendering default parameters useless.

router.on("route:init", (foo = "hamburger", bar = "vegan") => {
   console(foo, bar); // null, null
});

Solution would be passing no value at all or undefined instead of null, but I am not sure which browsers the comment about cross-browser inconsistencies refers to. The workarounds are rather trivial (and it might not be the best use-case in general either), so the issue is not that urgent by any means.

petetnt avatar Jan 12 '16 12:01 petetnt

We can change it to undefined instead, and that'll keep the cross-browser consistency. It's a mildly breaking change, though.

jridgewell avatar Jan 12 '16 16:01 jridgewell

Yup, it could have some nasty side effects for someone doing something like if (foo !== null). Maybe for the next major version then :fire_engine:

petetnt avatar Jan 12 '16 18:01 petetnt

In the meantime, #_extractParameters is meant to be overridden (though you wouldn't know it by "private" naming). You could copy/paste it in your own code, changing null to undefined.

jridgewell avatar Jan 12 '16 20:01 jridgewell

@jridgewell Cool, thanks for the tip!

petetnt avatar Jan 12 '16 20:01 petetnt