search

stripe / stripe-node

Node.js library for the Stripe API.
https://stripe.com
MIT License
3.9k stars 755 forks source link

feat: migrate to picoquery #2146

Open 43081j opened 4 months ago

43081j commented 4 months ago

Migrates from qs to picoquery for query strings, a faster and lighter alternative.

This is an alternative to #2116 - using pq to provide the logic rather than rolling our own. We are doing the same in storybook and many other repos, so figured it'd be worth doing so here too.

if you'd prefer to roll your own, feel free to close this

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

CLAassistant commented 4 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

helenye-stripe commented 3 months ago

Hi @43081j, thanks so much for this submission! Sorry for the delay. I think after reviewing the approaches for removing qs, we would like to investigate this further. Would you mind investigating the test failure here?

43081j commented 3 months ago

unfortunately this is because node 12 doesn't support nullish coalescing

bumping the engine constraint here (i.e. dropping node <14 support) seems the sensible thing to do eventually but that seems like a bigger decision for you to make

until then i guess this PR is blocked

let me know what you think. if we have to hold off until node <14 support is dropped one day, im happy to come back at that point and revisit this

xavdid-stripe commented 3 months ago

Yes, our version support policy is something we're working on. We'll put a pin in this and merge it when we can drop node 12 support.

Thank you for your contribution!