Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace deprecated url.parse with new URL #1927

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Steve-Mcl
Copy link

closes #1569

Alters internal processing of connection URL. Removes url.parse and uses new URL

This is done in a backwards compatible manor however I would recommend updating all types and function signatures to use the the URL object instead of a string (or as well as) but that may be more suited to a major bump.

NOTE: there are some TODOs in the code that need to be understood (and removed).

@Steve-Mcl Steve-Mcl changed the title Replace url.parse with new URL() fix: IPV6 parsing Aug 15, 2024
@Steve-Mcl
Copy link
Author

@robertsLando is there something I need to do to permit tests to run?

@robertsLando
Copy link
Member

@Steve-Mcl sorry I was on vacation till yesterday. I have to approve your PR in order to allow test to run, done now. I will also do a quick review

@robertsLando robertsLando changed the title fix: IPV6 parsing fix: replace deprecated url.parse with new URL Aug 28, 2024
@robertsLando
Copy link
Member

robertsLando commented Aug 28, 2024

I know there have been another try to do this in the past: #1147 then reverted #1217

Problem is that it has been reverted as it caused many compatibility issues. We need to track all possible issues related to this change as I'm pretty sure I will get lot of reports if/when I merge this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants