proxy-from-env icon indicating copy to clipboard operation
proxy-from-env copied to clipboard

WHATWG URL compliance

Open stevenvachon opened this issue 8 years ago • 7 comments

via universal-url

stevenvachon avatar May 30 '17 18:05 stevenvachon

Why?

Rob--W avatar May 30 '17 20:05 Rob--W

Because it's only going to get more popular with time. http/https modules support them in Node 8.x. They also parse at least 2x faster.

stevenvachon avatar May 30 '17 20:05 stevenvachon

My library is already compatible with WHATWG URLs, the exported function behaves identical regardless of whether the input is a WHATWG URL or a legacy (Node.js) URL. The return value is a string. If the consumer really wants to have a URL, it is trivial to do new URL(returnValueHere) to obtain a URL.

So, I don't see a benefit in adding "universal-url" as a dependency.

Rob--W avatar May 30 '17 21:05 Rob--W

I was referring to the internal use of url.parse(). It's better to use the specification parser.

stevenvachon avatar May 30 '17 21:05 stevenvachon

I was referring to the internal use of url.parse(). It's better to use the specification parser.

Better in what sense? I don't see a functional difference between the two parsers in my use case. I only use scheme (.protocol), host name (inferred from .host) and port (.port) properties of (parsed) URLs.

There must be a compelling reason before I add new dependencies. The only potential advantage that you've mentioned so far is a "at least 2x faster". I don't attribute much value to this claim, not only because of the lack of benchmarks, but mainly because the existing implementation is already sufficiently fast and unlikely to be a bottleneck for performance issues.

Rob--W avatar May 30 '17 21:05 Rob--W

The specification uses TR46 to parse IDNA host names.

Benchmarks: https://github.com/nodejs/node/pull/10678#issuecomment-272607729

stevenvachon avatar May 30 '17 22:05 stevenvachon

Should probably switch the legacy url to use URL, present as global since Node.js 10.0.0.

silverwind avatar Oct 16 '21 10:10 silverwind