-
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
Inconsistent path relativization #103
Conversation
What the hell, I'm working on this now. Here's a branch with a test case: https://github.com/djcsdy/URI.js/tree/issue-103 The problem is that URI.js can internally represent missing components as either
if (relativeParts.protocol === baseParts.protocol) { ... }
// null !== "" Two possible solutions here:
The first solution seems cleaner to me but the second is simpler to implement... |
I used to treat |
I pushed a fix to https://github.com/djcsdy/URI.js/tree/issue-103. It works by normalizing empty URI components to There seem to be some other cases where URI parts can be set to the empty string though, and in those cases |
do you have the time to investigate? |
Yeah, working on it now :). |
I turned this issue into a pull request, which should fix this bug. Empty URI parts are now normalized to |
Oops, we broke something in
relativeTo()
:(.Since the last release,
relativeTo()
behaves inconsistently for relative URIs that contain an absolute path but no scheme or authority.This works as expected:
This should be equivalent, but since the last release it produces a different result:
Which is curious because the two URIs to be relativized are equivalent:
Of course, path relativization and URI relativization are fundamentally different things. A URI that contains only a path component is already relative, so it's not entirely clear what the expected behaviour of
relativeTo()
should be. This is the sort of thing I was talking about when I said that the definition ofrelativeTo()
needs to be tightened up :).Of course, using
relativeTo()
to relativize an absolute path used to work, so it should continue to work.It's probably me that broke this, sorry about that. I need this functionality for a commercial project I'm working on, so I will certainly get around to fixing it sooner or later if you don't beat me to it. Possibly this week, but I'm not sure yet :).