normalize-url
normalize-url copied to clipboard
sindresorhus
Add `removePort` option
In the Docs, you have an example that has a port on a url and then when you wrap the url with "normalizeUrl" the port is no longer included:
When I run this same example locally, the Port :80/ is still included - which is different behavior from what your documentation example above shows:
How would I remove the :80/ port portion of a url with your package ?
I am pretty sure there is a bug because when I did another test the behavior is not consistent. As you can see in the image below, the results are not consistent just by changing a url from having http too https and also I see that just changing the actual number for the port in the url makes the behavior different too:
Why would it remove 80? The default port for HTTPS is 443. I think the existing behavior is correct.
Why would it remove
80? The default port for HTTPS is 443. I think the existing behavior is correct.
Hi, Thanks for your quick reply.
hmm, well in my case I wanted an option to remove the port so I can create a more user friendly/readable url. I used your library but had to do an additional step of regex to strip the :# port number from it.
I had assumed your library would have let me remove the port (any port) on the url, instead of it deciding when a port should be removed or kept.
Do you think that adding an option to remove a port (any port found) to your library would be a feature you could implement?
It could be added with the ability to pass in true to remove all ports or it can also accept regex, like how you set it up for "removeDirectoryIndex":
normalizeUrl('www.sindresorhus.com/foo/default.php', {
removeDirectoryIndex: [/^default\.[a-z]+$/]
});
it could be named something like "removePort"
Removing port 80 from a HTTPS URL would could potentially break the URL, although unlikely. I'm ok with a removePort: true option. I think a boolean is enough for now. If people have a need for more, they can request it.
Nice, I can work on this task.
Just to get some clarity before I make a pull request, when you said:
Removing port 80 from a HTTPS URL would could potentially break the URL, although unlikely.
When I add the removePort boolean option, I still make it remove all ports including port 80, right - since like you mentioned its unlikely to cause an issue?
@ianizaguirre
I think removePort should be rename to unsafeRemovePort, or current naming style forceRemovePort.
Because https://example.com:80 and https://example.com is NOT SAME AT ALL, they MAY just happen to LOOK same in specific cases.
- https://example.com:80 Secure Connection Failed
- https://example.com Example Domain
This suggestion may prevent confusion with the safe remove port, like remove :80 from http and remove :443 from https.
@sindresorhus
Pull request has been created: https://github.com/sindresorhus/normalize-url/pull/174