balazsbotond / urlcat

A URL builder library for JavaScript.
https://urlcat.org
MIT License
1.82k stars 57 forks source link

Query params output #103

Closed Saafine closed 2 years ago

Saafine commented 3 years ago

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 3 years ago

Hey @balazsbotond Can I help for this?

balazsbotond commented 3 years ago

@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 3 years ago

@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 3 years ago

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!!

harshilparmar commented 3 years ago

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

balazsbotond commented 3 years ago

@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.

harshilparmar commented 3 years ago

@balazsbotond I am on it πŸ‘¨β€πŸ’»

balazsbotond commented 3 years ago

@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 3 years ago

@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 3 years ago

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

balazsbotond commented 2 years ago

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

harshilparmar commented 2 years ago

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 ☺️

balazsbotond commented 2 years ago

@harshilparmar nice to see you again too :)