-
Notifications
You must be signed in to change notification settings - Fork 476
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
bug: cannot get domain of URI missing protocol #260
Comments
|
Looks due to https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L494 |
This seems like an unintuitive result for a very common scenario. |
I don't disagree, judging by the number of people who got stumped by this so far.
Since the developer likely knows what data they're dealing with, they could go with Here are a few quick thoughts
|
Maybe something like this:
where |
that's not going to work (easily) because at the point |
Sure, but we should be able to |
why knowingly parse the string twice? unless you expect |
Doesn't seem too expensive to do so, and makes the API clean. Alternatively, you could flip the order to only parse once:
which is also pretty clean. |
Mind re-opening this issue to get more pairs of eyes reviewing any proposals? |
Why do you consider |
I'm partial to the builder pattern for flexibility/future-proofing and readability. |
In that case I'd prefer to go the functional programming route: URI.withDefaults({ protocol: 'http' })('portquiz.net:8');
var httpUri = URI.withDefaults({ protocol: 'http' });
[ 'portquiz.net:8' ].map(httpUri); This would allow the API user to do the work only once. Now the next question (less question, more obstacle) is if the defaults transcend the var uri = URI.withDefaults({ protocol: 'http' })('portquiz.net:8');
uri.href('portquiz.net:9') I guess |
Sure, that sounds really reasonable. |
I'm running into this as well. I want to parse a list of URLs that may or may not contain protocols, it may even just be a path. Based on: https://medialize.github.io/URI.js/about-uris.html |
I'd love to see this implemented. |
I've implemented a possible solution using @MichielVanderlee 's suggestion. This allows me to now do We'll see if it's worthy of being pulled in, but for now, it's working for my needs. |
The text was updated successfully, but these errors were encountered: