refactor: remove deprecated url.parse() in favor of WHATWG URL API#2470
refactor: remove deprecated url.parse() in favor of WHATWG URL API#2470
url.parse() in favor of WHATWG URL API#2470Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes URL parsing by replacing the deprecated Node.js url.parse() method with the WHATWG URL API across test files. This change improves code maintainability and ensures compatibility with current JavaScript standards.
- Replaced
url.parse()withURLconstructor for parsing URLs - Simplified URL property access using WHATWG URL API properties
- Removed unnecessary test environment setup code in tracks test
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
__tests__/lib/validations/is-multisite-domain-mapped.js |
Updated URL parsing in test setup to use WHATWG URL API |
__tests__/lib/analytics/clients/tracks.js |
Refactored URL parsing and simplified buildNock function, removed environment setup code |
Comments suppressed due to low confidence (2)
tests/lib/analytics/clients/tracks.js:7
- The variable name 'url' conflicts with the removed 'url' module import and may be confusing. Consider renaming to 'endpointUrl' or 'tracksUrl' for clarity.
const url = new URL( Tracks.ENDPOINT );
tests/lib/validations/is-multisite-domain-mapped.js:82
- The variable name 'url' conflicts with the removed 'url' module import and may be confusing. Consider renaming to 'apiUrl' or 'parsedUrl' for clarity.
const url = new URL( API_URL );
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
| const folder = arg.join(); // File comes in as an array as part of the args- turn it into a string | ||
| arg = url.parse( folder ); // Then parse the file to its URL parts | ||
| const filePath = arg.path; // Extract the path of the file | ||
| const filePath = arg.join(); // File comes in as an array as part of the args- turn it into a string |
There was a problem hiding this comment.
@ariskataoka, do you happen to remember why we used url.parse() here?
As far as I understand, folder is a path on a user's file system, such as ~/user/path/to/uploads; it is never a URL. Why do we attempt to parse it as a URL?
| /** | ||
| * Allow overriding of process variables per test | ||
| * Adapted from https://stackoverflow.com/a/48042799 | ||
| */ | ||
| const OLD_ENV = process.env; | ||
|
|
||
| beforeEach( () => { | ||
| jest.resetModules(); | ||
| process.env = { ...OLD_ENV }; | ||
| } ); | ||
|
|
||
| afterEach( () => { | ||
| process.env = OLD_ENV; | ||
| } ); |
There was a problem hiding this comment.
Environment overrides are not used in tests.
|
/changelog |
AI-Generated Changelog EntryImprove compatibility and reliability by updating URL handling to use modern standards instead of outdated methods; this change has no visible effect for most users. |
sjinks
force-pushed
the
fix/url-parse-deprecation
branch
from
6b24c85 to
a32bed1
Compare
|
Description
This pull request refactors test files to replace the deprecated
url.parsemethod with the modern WHATWG URL API for parsing and handling URLs. This change simplifies the code and ensures compatibility with the current JavaScript standards.Refactoring to use the WHATWG URL API:
__tests__/lib/analytics/clients/tracks.js: Replaced theurl.parsemethod with theURLconstructor to parseTracks.ENDPOINTand refactored thebuildNockfunction to useurl.originandurl.pathname.__tests__/lib/validations/is-multisite-domain-mapped.js: Removed theurlimport and updated theisMultisitePrimaryDomainMappedtest to use theURLconstructor for parsingAPI_URLand constructing thenockrequest. [1] [2]Changelog Description
Fixed
Pull request checklist
New release checklist
Steps to Test
CI must pass.