-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
scheme-relative is absolute? #2
Comments
Why? |
Because |
Unless those slashes denote an internal network host. In which case, such logic should be toggle-able and probably not default. See |
Hmm, good question. You could argue both ways for it. Yes, it's protocol-relative, but it's still an URL that resolves to an absolute resource, not a relative path. // @kevva @shinnn @floatdrop @stevemao Thoughts? |
Seems to be an absolute to me but @stevenvachon has a very good argument.. |
Personally I'd go for |
In this case I expect is-absolute-url to return A new option meets both requirements? // Just a casual idea
isAbsoluteUrl('//github.com', {includeProtocolRelative: false}); //=> false
isAbsoluteUrl('//github.com', {includeProtocolRelative: true}); //=> true |
Also, just look at the name: protocol-relative, scheme-relative. All URLs resolve to absolute when they have a base URL. |
If you type |
@stevenvachon It's absolute in the sense that it's not a relative path. What's your use-case needing it to be something different? I think the current behaviour is the most sensible, and unless you have a good use-case, I would say it only needs doc clarification. |
Parsing URLs in HTML for example wouldn't know whether to use http or https unless |
Hmm... From Python docs:
But then from StackOverflow:
I guess it makes more sense to follow the latter as it's from the |
The URL Standard is split across HTML and URL specs. It's kinda messed up. |
Both is correct here imo, but I would expect |
Maybe this could return |
No. |
// isAbsolute(url, slashesDenoteHost);
isAbsolute("//google.com/"); //-> fase
isAbsolute("//localhost/", true); //-> true
isAbsolute("localhost/", true); //-> false
|
Boolean arguments is nasty. |
isAbsolute("/google.com/"); //-> isAbsolute.RELATIVE === 0
isAbsolute("http://google.com/"); //-> isAbsolute.ABSOLUTE === 1
isAbsolute("//google.com/"); //-> isAbsolute.PROTOCOL_RELATIVE === 2 also pretty nasty |
I was thinking maybe the return value couldn't be a simple |
Consider this situation. |
See my comments at nodejs/node#1790 Point is that io.js does not have any bug and |
So, conclusion is that it shouldn't be treated as a absolute URL? |
The spec says that
What happens when another hiararchical URI is not specified. I personally, and in case of Node environment would parse it as local file path; meaning that is actually is badly formatted but functionally valid absolute path (at least in POSIX environments). But when used with |
If singular For reference I just tested with Python So |
The intent of this repo is to check if an URL is absolute so in this case we should return false. |
Exactly. |
Do note however, that local file paths can also be URL ;) |
So if this library chooses it can treat URLs without protocol given as being For example browsers default to And then there is the option is to demand a specific protocol for all inputs and fail if its not provided. |
@stevenvachon Would you be able to do a doc update for #3? |
Regex taken from http://stackoverflow.com/a/31991870/182702 and modified to disallow URIs starting with "//" (scheme-relative URIs) per sindresorhus#2.
https://github.com/sindresorhus/is-absolute-url/blob/master/test.js#L9
I'd think that it should be
false
.The text was updated successfully, but these errors were encountered: