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

bug: cannot get domain of URI missing protocol #260

Open
jklmli opened this issue Oct 26, 2015 · 18 comments
Open

bug: cannot get domain of URI missing protocol #260

jklmli opened this issue Oct 26, 2015 · 18 comments

Comments

@jklmli
Copy link

jklmli commented Oct 26, 2015

> URI('portquiz.net:8').domain()
< ""
@jklmli
Copy link
Author

jklmli commented Oct 26, 2015

host() and port()also don't work.

@jklmli
Copy link
Author

jklmli commented Oct 26, 2015

> URI.parse('portquiz.net:8')
< Object {protocol: "portquiz.net", urn: true, path: "8"}

Looks due to https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L494
What protocols include .+-? https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L199

@rodneyrehm
Copy link
Member

yes, that's known behavior - not a bug. see #232 or #187 for details.

@jklmli
Copy link
Author

jklmli commented Nov 5, 2015

This seems like an unintuitive result for a very common scenario.
Maybe have URI() and URI.parse() ignore the literal spec, and introduce a new function URI.strictParse() which follows it? Having every user write a preprocessing method is a bit wasteful.

@rodneyrehm
Copy link
Member

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.

Maybe have URI() and URI.parse() ignore the literal spec, and introduce a new function URI.strictParse() which follows it? Having every user write a preprocessing method is a bit wasteful.

Since the developer likely knows what data they're dealing with, they could go with URI.parseAsHttp(input) instead of URI(input) to state their expectation of working with an HTTP/S URL. We're certainly not changing the way URI works right now, but we can easily add this feature in a non-breaking way. Should we start by bikeshedding the API? Do you want to implement this and send a PR after discussions?

Here are a few quick thoughts

  1. URI.parseAsHttp(input) with leads to a number of new functions if we were to add support for more protocols
  2. because of the existing signature of URI.parse(inputString, outputMap), we can't easily add an options object as another argument. I'm not sure I like URI.parse({uri: inputString, option1: true}, outputMap) to get around that
  3. since URI.parse is an internal method and the proposed new function an external method (i.e. it is not ever used internally, only called by the developer to initiate working with URI), we could go with URI.parseWithDefault(inputString, optionsMap). This way optionsMap could contain defaults for all the components, not only the protocol. This looks dangerously close to URI(inputString, relativeBase), though.

@jklmli
Copy link
Author

jklmli commented Nov 6, 2015

Maybe something like this:

> URI.parse('portquiz.net:8').withDefaults({protocol: 'http'})
< Object {protocol: "http", domain: "portquiz.net", port: "8"}

where withDefaults returns a new URI object?

@rodneyrehm
Copy link
Member

URI.parse('portquiz.net:8').withDefaults({protocol: 'http'})

that's not going to work (easily) because at the point withDefaults() is evaluated, the "wrong interpretation" of your input string has already happened.

@jklmli
Copy link
Author

jklmli commented Nov 6, 2015

Sure, but we should be able to toString the URI object to get back the original string, and re-parse from there with the defaults.

@rodneyrehm
Copy link
Member

Sure, but we should be able to toString the URI object to get back the original string, and re-parse from there with the defaults.

why knowingly parse the string twice? unless you expect .withDefaults() to not engage very often (which is an assumption I'm not ready to share).

@jklmli
Copy link
Author

jklmli commented Nov 6, 2015

Doesn't seem too expensive to do so, and makes the API clean.

Alternatively, you could flip the order to only parse once:

URI.withDefaults({protocol: 'http'}).parse('portquiz.net:8')

which is also pretty clean.

@jklmli
Copy link
Author

jklmli commented Nov 6, 2015

Mind re-opening this issue to get more pairs of eyes reviewing any proposals?

@rodneyrehm
Copy link
Member

Why do you consider URI.withDefaults({protocol: 'http'}).parse('portquiz.net:8') better than URI.parseWithDefaults('portquiz.net:8', {protocol: 'http'})?

@rodneyrehm rodneyrehm reopened this Nov 6, 2015
@jklmli
Copy link
Author

jklmli commented Nov 6, 2015

I'm partial to the builder pattern for flexibility/future-proofing and readability.
A lot of good discussion here which I'd do a disservice to if I were to try to summarize.

@rodneyrehm
Copy link
Member

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 URI.parse() method, so that the following behaves the same way:

var uri = URI.withDefaults({ protocol: 'http' })('portquiz.net:8');
uri.href('portquiz.net:9')

I guess URI.withDefaults() would work like a factory, bootstrapping the URI "class" so that any instance of that customized URI behaves consistently. We've dealt with this in an ugly and global way in the past, as URI.escapeQuerySpace can attest.

@jklmli
Copy link
Author

jklmli commented Nov 25, 2015

Sure, that sounds really reasonable.

@mvanderlee
Copy link

mvanderlee commented May 2, 2018

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.
i.e.:
['http://hostname:port/path?query', 'hostname:port', '/path']

Based on: https://medialize.github.io/URI.js/about-uris.html
It would think we could even have a flag that will ensure we always parse a URL instead of a URN?
Or simply URI.parseURL(urlString) so I could do new URI(URI.parseURL(urlString))

@DoDSoftware
Copy link

I'd love to see this implemented.

@DoDSoftware
Copy link

I've implemented a possible solution using @MichielVanderlee 's suggestion.

#374

This allows me to now do let uri = new URI(URI.toUri('somesite.com', {scheme: 'http'}));
after which, uri.domain() will return the expected somesite.com

We'll see if it's worthy of being pulled in, but for now, it's working for my needs.

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

No branches or pull requests

4 participants