feat(util): use node:querystring for buildURL #1677
Conversation
Codecov ReportBase: 94.74% // Head: 94.72% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1677 +/- ##
==========================================
- Coverage 94.74% 94.72% -0.02%
==========================================
Files 53 53
Lines 4892 4876 -16
==========================================
- Hits 4635 4619 -16
Misses 257 257
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
the tests were checking error messages that were no longer applicable, I added tests for a bunch of types as a replacement. I could add them back but i'm not sure how useful they would be |
|
LGTM assuming we are pretty sure this is not breaking |
|
@KhafraDev Can you rebase your branch? |
|
I'll open a new pull request to see the benchmark results for before/after |
* feat(util): use node:querystring for buildURL * fix: skip test on v12 * fix: handle case for empty object
* feat(util): use node:querystring for buildURL * fix: skip test on v12 * fix: handle case for empty object
Refs: #1673
Fixes: #1672
There is very little performance difference between node's querystring and fast-querystring and it's very unlikely that
buildURLis a performance bottleneck.This pr is technically a breaking change as objects and undefined params are now allowed (null, undefined, objects, functions, etc.) and no longer throw an error.