veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
201 stars 30 forks source link

Method params types suggestion #104

Closed harveysanders closed 5 months ago

harveysanders commented 1 year ago

Hi! Just a suggestion: Most of the FlowRouter methods have optional params.

interface Router {
    // ....
    url: (path: string, params?: Param, qs?: QueryParam) => string;
    path: (path: string, params?: Param, qs?: QueryParam) => string;
    // ....
 }   

With these types, one needs to pass undefined or an empty placeholder value.

const path = FlowRouter.path('a-path', undefined, {q: "a query"});

image

In my opinion, undefined represents values that are not yet set in the runtime, whereas if you explicitly need to set a value to a non-value, null should be used. With that said, I think it may make sense to use union types with null instead of the question mark so devs call the methods like this:

const path = FlowRouter.path('a-path', null, {q: "a query"});

If you agree, I could submit a PR at some point.

Thanks for your time and work on this project!

dr-dimitru commented 7 months ago

Hello @harveysanders,

Pardon me for answering almost a year later, I agree, and love to add support for null along with void 0/undefined feel free so send a PR

harveysanders commented 6 months ago

@dr-dimitru no worries! I just submitted #106 for your consideration

dr-dimitru commented 5 months ago

Published as v3.10.1 @harveysanders thank you for contribution 🙏