twilio / twilio-node

Node.js helper library
MIT License
1.37k stars 495 forks source link

feat: move to built-in URL class #998

Closed 43081j closed 3 months ago

43081j commented 5 months ago

We currently use url-parse to deal with URL construction. Given our node engine constraint of >=14, we know URL is available in all supported node versions (introduced in 10.x if i remember correctly).

So we should be able to drop this dependency and use the built-in parser instead.

Note that one test here changed hash because url-parse incorrectly did not encode the single quotes in the test URL. With the standard URL parsing, it is now encoded.

Let me know if there's anything you want me to change here. Also, if someone could confirm i updated the test hash correctly, that'd be very helpful.

Checklist

tiwarishubham635 commented 3 months ago

Hi @43081j! Can you try running make prettier in your local repository before pushing? That should fix the this error.

tiwarishubham635 commented 3 months ago

I have added your changes in a new branch to monitor the development. Please check here. Thanks!

43081j commented 3 months ago

no worries, thanks for picking this up!

do you want to close this one? since your PR will land the changes anyway

tiwarishubham635 commented 3 months ago

no worries, thanks for picking this up!

do you want to close this one? since your PR will land the changes anyway

Yeah, i think we can proceed from this PR now

tiwarishubham635 commented 3 months ago

We can close this PR now, please take a look at #1017 and let us know if everything is good. Thanks!