balazsbotond/urlcat

Query params output

Saafine opened this issue · 13 comments

Saafine commented

Describe the bug
Possibly incorrect queryParams output

To Reproduce
urlcat('https://google.com', '', {test: 'abc'})
returns https://google.com/test=abc

Expected behavior
Should return
https://google.com?test=abc

harshilparmar commented

Hey @balazsbotond Can I help for this?

balazsbotond commented

@Saafine thank you for the bug report.

I've successfully reproduced it using the currently published npm version and it is indeed a problem.

balazsbotond commented

@harshilparmar thanks, that would be great. I have a lot of work to do these days so I'm glad you showed up.

As a first step, we need to reproduce the bug on both 2.x and the master branch - let's see which version it affects. Can you help with this first? After that, let's discuss the results here and decide how to proceed.

harshilparmar commented

As a first step, we need to reproduce the bug on both 2.x and the master branch - let's see which version it affects. Can you help with this first? After that, let's discuss the results here and decide how to proceed.

@balazsbotond Yeah sure, I will let you know!!

  • 👍1
harshilparmar commented

@balazsbotond brother, this issue affects in both 2.x and master.
Observation : I think urlcat second param takes empty string in consideration.

balazsbotond commented

@harshilparmar thank you! I'm sorry it took this long to reply. If you still have the time, I'd be very thankful if you could work on a fix.

  • 🚀1
harshilparmar commented

@balazsbotond I am on it 👨‍💻

  • 👍1
balazsbotond commented

@harshilparmar if you have time, could you backport the fix to the 2.x branch and send another PR against that branch? This way I could release a new patch release after merging it and the latest published version would be free of known bugs.

harshilparmar commented

@harshilparmar if you have time, could you backport the fix to the 2.x branch and send another PR against that branch? This way I could release a new patch release after merging it and the latest published version would be free of known bugs.

Sure brother 👍🏼

harshilparmar commented

@balazsbotond Can you please close this issue? It might take time for new contributors.

balazsbotond commented

Closing because I decided to delete the 2.x branch and concentrate on maintaining the currently published version.

harshilparmar commented

Closing because I decided to delete the 2.x branch and concentrate on maintaining the currently published version.

Really happy to see you back here ☺️

  • 👍1
balazsbotond commented

@harshilparmar nice to see you again too :)

  • 👍1