Clean up the parameters that are passed to the handler #114
Conversation
| // If the route does match, then collect values for all the named | ||
| // parameters that were returned in keys. | ||
| // If there are no named parameters then this will end up returning {}, | ||
| // which is truthy, and therefore a sufficient return value. |
There was a problem hiding this comment.
What happens to the params value when there is no capture groups? Previously it was true which didn't feel like the right value given what it represents.
There was a problem hiding this comment.
As per the inline comment in the code:
If there are no named parameters then this will end up returning {}...
That will result in params being set to {}, which seemed reasonable to me. Do you think a different value would make more sense? It needs to be something truthy.
There was a problem hiding this comment.
Personally I'd expect params to be undefined if the route defines no routes. The match method needs to be truthy, but the callback to the developer facing API could have this filtered out if the value is not an object, meaning you could return true and it would get parsed out.
There was a problem hiding this comment.
Okay, I've updated the code to behave the way you describe.
|
Travis is failing because the new |
|
Merging now that Travis CI is happy. It's always a good sign when tests break when interfaces change 😄 |
R: @addyosmani @gauntface
Fixes #107
This leads to less cruft being passed along to the handler's
paramswhen using theRegExpRouteorExpressRouteclasses along with capture groups/named parameters.I've got some tests for this module in flight at https://github.com/GoogleChrome/sw-helpers/tree/sw-routing-tests and I'll add tests for this modified functionality as part of a PR for that branch.