Skip to content

Conversation

@dflynn15
Copy link

@dflynn15 dflynn15 commented Jan 2, 2026

Summary

  • Removed import url from 'url'
  • Replaced url.parse(requestUrl) with new URL(requestUrl)
  • Updated getRequestOptionsFromUrl() to use the WHATWG URL interface:
    • Changed parameter type from url.UrlWithStringQuery to URL
    • Changed url.path → url.pathname + url.search
    • Changed empty port handling to return null (matching original behavior)

url.parse() has been deprecated since Node.js 11. The listed support, Node.js 18+ (per package.json), means we should use the modern WHATWG URL API which is globally available.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the deprecated url.parse() API (deprecated since Node.js 11) and replaces it with the modern WHATWG URL API, which is globally available in Node.js 18+.

Key Changes:

  • Replaced url.parse(requestUrl) with new URL(requestUrl) constructor
  • Updated getRequestOptionsFromUrl() to accept WHATWG URL objects and adapted property access patterns
  • Removed the deprecated url module import

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

path: url.path,
port: url.port,
path: url.pathname + url.search,
port: url.port || null,
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The port handling logic is incorrect. The WHATWG URL API's port property returns an empty string ("") when no explicit port is specified, not a falsy value. Using url.port || null will incorrectly set the port to null even when a valid port exists if the port is "0", and will correctly handle empty strings but for the wrong reason.

The correct approach is to check for an empty string explicitly: url.port ? url.port : null or url.port !== '' ? url.port : null. This ensures that:

  • Valid ports (including "0") are passed through
  • Empty strings (no explicit port) become null
  • The http.RequestOptions receives the appropriate value
Suggested change
port: url.port || null,
port: url.port !== '' ? url.port : null,

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant