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

go() query string repetition bug #93

Closed drone1 closed 2 years ago

drone1 commented 2 years ago

I'm having an issue:

const route = FlowRouter.current()
const qs = {...route.queryParams} // clone
qs.test = 1
FlowRouter.go(
    route.path,
    {...route.params},  // clone
    qs
)

Shouldn't this be like calling setQueryParam()? Instead it results in the current path etc but adds ?test=1?test=1?test=1... to the URL. I would expect /path?test=1

zeearth commented 2 years ago

@dr-dimitru hi, your commit generate a bug we have more than one parameter optional into pathDef. Our pathdef : '/companies/:id?/:action?/' When we call the path method : FlowRouter.path('companies', { id: this._id._str, action: mode }) the generated path is no more correct : "/companies/6135cb32d14df059605901fd?%2F%3Aaction="

in 3.7.5 of the package the generated path is like this : "/companies/6135cb32d14df059605901fd/edit"

thanks for you return

dr-dimitru commented 2 years ago

Hello @zeearth thank you for reporting.

This case is part of the tests.

Can you post values of console.log({id: this._id._str, action: mode}) looking ant the generated path mode is null or empty string?

zeearth commented 2 years ago

Hello @dr-dimitru

I saw your test, but my parameters are optionnal. I'm thinking problems come from the split on the '?' router.js:226 I think the split is done for separate uri and query string. this._id._str is a common mongodb id string "629f87f44ec0e563c4d5c458" mode is a string view or edit.

dr-dimitru commented 2 years ago

@zeearth seems like I finally caught this one, try running on updated v3.8.1. lmk

zeearth commented 2 years ago

Good for us @dr-dimitru. Thanks for the quick buck fix and for all you work.