-
Notifications
You must be signed in to change notification settings - Fork 475
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
.equals(undefined) #189
Comments
that's because URI(location.href).equals(URI(location.href)) === true;
URI(location.href).equals(URI()) === true;
URI(location.href).equals() === true;
URI().equals(URI(location.href)) === true;
URI().equals(URI()) === true;
URI().equals() === true; I don't consider this a bug. Maybe the documentation could be more clear about it… |
While all of those make sense URI(undefined).equals(URI()) === true doesn't really. It is very easy to make mistakes by inputting something you think is a value (but is really undefined) to URI. This is not a logical behavior. A possible solution would be adding if (arguments.length === 1) {
throw new TypeError('undefined is not a valid argument for URI');
} to line 43. But obviously it is your call! |
(I took the liberty of fixing your code examples and make line 43 be 100x more useful by linking it to the mentioned line…) The constructor takes 2 arguments, so you'd likely check
Actually it's the community's, as your suggested change introduces a (tiny) backward compatibility break (which I assume most people wouldn't run into): // currently equivalent
URI(undefined);
URI(location.href); But I guess that's fine, considering that you suggested throwing an error instead of silently using the empty string. |
@eakron do you want to send a PR to fix this issue? |
Pull request: #196 |
If I am on the page "http://local.local" and run
URI('http://local.local').equals(undefined)
I gettrue
. Comparing a different URL to undefined gives false. As far as I can tell, this only happens if the argument toURI
is the current page URL.I assume this is so that
URI()
is a shortcut for current URL. This can be differentiated by looking atarguments
. Will make a pull request.Using version 1.14.1.
The text was updated successfully, but these errors were encountered: