-
-
Notifications
You must be signed in to change notification settings - Fork 33
feat: implement custom URL parser to replace deprecated url.parse #44
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
base: master
Are you sure you want to change the base?
Conversation
|
Hey @jiji-hoon96! Thanks for investing time and effort in helping the project! ❤️ You are right the var url = require('url')
var parse = url.parse
console.log(parse('https://expressjs.com/en/4x/api.html#req.protocol'))I was wondering if you explored the option to use the The output is quite similar, but as mentioned here ( The regex you proposed in your parser seems to follow a linear complexity, but I prefer to avoid include custom implementations if we can use built-in utils or existing APIs. wdyt @blakeembrey? |
|
Thanks for the suggestion! You're right - using the |
|
Given the constraints, I believe is the best available option because
However, I'm open to any direction you'd prefer. Should we
Let me know your thoughts! 🙏 |
Proposal: Modernizing parseurl by Embracing Native Node.js APIsHello everyone,
Implementation PreviewI have worked on a draft implementation of this change:
Impact Analysis: Backward Compatibility is Key This update is designed to be fully backward compatible. Because the output retains the expected API structure as a plain object, existing consumers of the package should not require any modifications, ensuring a smooth transition for the entire ecosystem. Critical CI/CD Pipeline FixesIn parallel with the feature update, I've addressed critical maintenance issues in the existing CI/CD workflow (ci.yml) to ensure reliable testing on modern GitHub Actions runners:
CI/CD Fixes Implementation
I'll like to know what you think. |
- Remove legacy Node.js versions (0.8, 0.10, 0.12) and EOL versions - Keep only active LTS versions: 16.x, 18.x, 20.x, 22.x - Replace deprecated ::set-output with $GITHUB_OUTPUT syntax - Upgrade actions/setup-node to v4 (replacing custom nvm script - Upgrade coverallsapp/github-action from @master to @v2 - Remove version-specific dependency overrides no longer needed
|
Thank you for the constructive feedback! I have updated the PR to address the comments and fix the CI failures:
The CI checks are now passing. Ready for review! |
|
Nice 💜, To remove the deprecated |
Node.js v24 has deprecated
url.parse(), causing deprecation warnings. This is related to security issues (hostname spoofing) and will be removed in future versions.This PR replaces the deprecated
url.parse()with the WHATWG URL API (new URL()) as recommended by Node.js, avoiding custom regex implementations for better reliability and maintainability.Changes
url.parse()with native WHATWG URL API (new URL())http://localhost) for compatibilityPerformance
Benchmark results show performance is maintained or improved compared to the original: