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

Allow WHATWG URL objects in interceptors #2435

Closed
2 tasks done
DesignByOnyx opened this issue Jan 4, 2023 · 2 comments · Fixed by #2437
Closed
2 tasks done

Allow WHATWG URL objects in interceptors #2435

DesignByOnyx opened this issue Jan 4, 2023 · 2 comments · Fixed by #2437

Comments

@DesignByOnyx
Copy link
Contributor

Please avoid duplicates

Context

It would be nice to allow WHATWG URL objects (implemented in node's URL API since v6.13.0, globally available since v10.0.0).

Since we are here, it would be nice to get rid of the legacy url.parse and simply use new URL(...) instead.

This may be a breaking change due to the fact that the legecay parser is slightly different than the WHATWG parser. I do not know what the differences are, but I imagine they are subtle and that some users have come to rely on the legacy implementation.

Alternatives

I can stringify URLs - but these strings are parsed... which is unnecessary.

// this works, but the stringification is not necessary
const apiServer = new URL('http://localhost:8888');
nock(`${apiServer}`)...

If the feature request is accepted, would you be willing to submit a PR?

  • yes
@DesignByOnyx
Copy link
Contributor Author

DesignByOnyx commented Jan 4, 2023

I took a stab at implementing this and wanted to post my initial findings. After digging through tests, I eventually found an attempt was made several years ago and it never got completed: #1692. The conversation in that PR highlights the biggest issue I've run into, but I'll recap here:

There is an unfortunate difference between the legacy url.parse and the WHATWG parser (new Url) regarding the hostname for bracketed IPv6 addresses:

const urlString = 'http://[2607:f0d0:1002:51::5]:8080';
url.parse(urlString).hostname //-> 2607:f0d0:1002:51::5
new URL(urlString).hostname //-> [2607:f0d0:1002:51::5]

Note the preservation of brackets in the WHATWG URL. This presents a few challenges:

  • preserving backwards compatability
  • treating [2607:f0d0:1002:51::5] and 2607:f0d0:1002:51::5 as equals... they're indeed the same hostname

Long story short, all of the above challenges are "solved" by keeping internal references to the non-bracketed hostnames - effectively preserving the legacy parsing behavior while still using the WHATWG parser and all of it's robustness. Here is some pseudo code:

const parsed = new URL(inputString);
const normalizedHostname = removeBrackets(parsed.hostname);
this.internalRef = { ...parsed, normalizedHostname }

All internal matching and filtering should take place on the normalizedHostname. This should be fairly easy to implement, and I will port over some of the other changes made the previous PR.

@github-actions
Copy link

🎉 This issue has been resolved in version 13.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

1 participant