Skip to content

Replace parseurl with WHATWG URL API#315

Open
Saturate wants to merge 4 commits intoeugef:masterfrom
Saturate:chore/replace-parseurl-dependency
Open

Replace parseurl with WHATWG URL API#315
Saturate wants to merge 4 commits intoeugef:masterfrom
Saturate:chore/replace-parseurl-dependency

Conversation

@Saturate
Copy link
Copy Markdown

More information here:

e18e/ecosystem-issues#160

@Saturate Saturate changed the title chore: Replace parseurl with WHATWG URL API Replace parseurl with WHATWG URL API Feb 19, 2025
@github-actions
Copy link
Copy Markdown

Stale pull request message

@eugef eugef requested a review from Copilot April 27, 2025 01:39
Copy link
Copy Markdown

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 replaces the use of the deprecated parseurl package with the WHATWG URL API to obtain the pathname from the request object.

  • Removed dependency on parseurl
  • Updated logic for retrieving the request pathname using the WHATWG URL API
Files not reviewed (1)
  • package.json: Language not supported

Comment thread lib/express/mock-request.js Outdated

defineGetter(req, 'path', function path() {
return parse(this).pathname;
return new URL(this).pathname;
Copy link

Copilot AI Apr 27, 2025

Choose a reason for hiding this comment

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

Using 'new URL(this)' may throw an error if 'this' does not contain an absolute URL. Consider providing a valid absolute URL string or a base URL as the second parameter to ensure proper parsing.

Suggested change
return new URL(this).pathname;
const baseUrl = `${this.protocol || 'http'}://${this.host || 'localhost'}`;
return new URL(this.url, baseUrl).pathname;

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown

Stale pull request message

@Saturate
Copy link
Copy Markdown
Author

Stale pull request message

Hmm, ok? :)

@github-actions github-actions Bot closed this Jul 5, 2025
@eugef eugef reopened this Jul 7, 2025
@github-actions github-actions Bot closed this Jul 15, 2025
@eugef eugef reopened this Jul 15, 2025
@github-actions
Copy link
Copy Markdown

Stale pull request message

@Saturate
Copy link
Copy Markdown
Author

Do you need something from me to make this PR easier to close?

@eugef
Copy link
Copy Markdown
Owner

eugef commented Sep 16, 2025

Hi @Saturate

Copilot actually caught a real issue here.

  • this isn’t a URL, it’s a request object, so you should use new URL(this.url).

  • Also, this.url might not always be an absolute URL. parseurl handled relative ones fine, but the URL constructor doesn’t. Adding a base URL (like Copilot suggested) will make sure relative URLs work correctly and avoid potential problems.

Example:

const req = {
    url: '/some/path?name=foo',
}

console.log(parse(req).pathname); // Output: '/some/path'
console.log(new URL(req.url).pathname); // TypeError: Invalid URL

@github-actions
Copy link
Copy Markdown

Stale pull request message

@github-actions
Copy link
Copy Markdown

Stale pull request message

@eugef eugef reopened this Apr 30, 2026
The URL constructor requires an absolute URL string, not a request
object. Use this.url with a constructed base URL to handle relative
paths like '/some/path?name=foo' that parseurl previously handled.

Also fix pre-existing eslint no-extraneous-dependencies error in
type declaration tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants