Handle relative URLs

[ashsearle]

Fixes #129, enabling router to intercept a tags with relative URLs, as well as enabling route() to handle relative URLs

An alternative to the resolve function implemented would be to resolve URLs using something like:

const a = document.createElement('a');
a.setAttribute('href') = url;
const resolvedPath = a.pathname;

...the problem I had with that was putting appropriate guards in place to make it work in a pure node environment.

[developit]

Hi @ashsearle - it might be possibly to dramatically simplify this by just changing .getAttribute('href') to .href, which is already nicely resolved by the browser. Thoughts?

[ashsearle]

Hi, that'd work for routeFromLink where you're starting with an anchor element, but wouldn't help with manual calls to route() (which may become common once pull request #100 is merged.)

[developit]

@ashsearle Good point. That said, what about using <a> to do the resolution? Probably still quite a bit smaller, and avoids having to bundle a parser and stuff into here (seems good):

let a = typeof document!=='undefined' && document.createElement('a');
function makeAbsolute(url) {
  if (a) {
    a.setAttribute('href', url);
    url = a.href;
  }
  return url;
}

[developit]

Implementation is good, but if it's only ever called from getCurrentUrl(), let's just inline it there.

[ashsearle]

It's also called from the new resolve method used by route.

I was wondering whether to tweak resolve to take a base parameter as well, then move it to util.js and add test-cases.

[developit]

Depends - there was talk about moving customHistory to be a property of Router instances so that they had proper instance separation instead of the singleton stuff. Might be better to go that route rather than making it more singelton-ey? Different PR I guess though.

[developit]

Any thoughts on the link parsing stuff? Seems like a size tradeoff, but the benefit of your custom parser is that it works under Node where document isn't available.

[ashsearle]

A few thoughts:

• do relative routes need to work on node? (Wondering if server-side rendering always takes a single pass through, rendering components and never re-routing...?)
• current resolve implementation isn't perfect - I'll work on a bug-fixing if you decide you want node support

If you're not worried about node support then going the <a> route makes a lot of sense as it does resolving and makes each part of the URL individually accessible (a.pathname, a.search etc.) - this is good for when you want to check protocol and hostname to weed-out absolute URLs

Is there a minimum version of node you support? I'm wondering if it'd make most sense to check for the URL API first (also in node), then fallback to a (for IE), and then... give up?

[developit]

SSR is always single-pass, yeah. Anything else would require a document shim/polyfill, which would give us resolution via <a>. I'd be inclined to keep things simple and use <a> for now, we can just document that relative links are not supported under node. I think that makes sense too - it would be very odd to have a global URL value in a nodejs instance, since node would typically be a server process handling many clients (even concurrently).

[ashsearle]

I've switched code over to use <a> for URL resolution, and tweaked the protocol/host checks to address an issue with custom histories (which may not record/use host or protocol.)

... unfortunately I think I may have screwed the pooch with poor git management. Perhaps I should start again with a clean master?

[developit]

Nah, I'm not a pedant about git, doesn't matter to me at all.

-edit- I think I get what you mean - fine by me if you want a clean one.

[developit]

Hmm - did these get added from another PR? I don't remember them being here before. .some() and .reduce() aren't supported in some of the browsers we support, and add to the overall size.

[ashsearle]

Sort of: I cocked-up putting pull request #136 on master, and brought them in here with poor git skills. I can resubmit.

Please check pull request #136 again: these methods (some and reduce) have been supported since way back in IE9, so should be fine... No polyfill bloat required.

[developit]

Good to know, I often forget these things now haha. Still curious about the filesize hit for all those function definitions, but I guess this repo isn't exactly aiming for extreme small size.

I'll pull this down this weekend and play around with it - I feel horrible about how long this PR and the nested routers PR have been sitting here, really want to get things merged since you guys did so much work on them!

[developit]

Tried to resolve conflicts with #169, hopefully I didn't mess anything up.

[developit]

Had a thought as I was resolving: there are kinda two cases here, and it might be a worthy exercise to think of them as different.

The first case is being able to invoke route('./b') while at the URL /foo/a, and end up at /foo/b. Personally I've not come upon a need for that, though I'm by no means an exhaustive user here.

The second case, and in my head I'm thinking this is the more common one, is that there is an anchor tag in a document that itself is bound to a URL, and we'd like to use its resolved href instead of the explicit attribute value of href. So <a href="b"> in a document found at /foo/a should link to /foo/b.

Interestingly, in splitting these two up, I've realize that the second case is actually something we can get for less than free (saves a few bytes) by simply switching from getAttribute('href') to .href in routeFromLink(). We could also switch target for some more bytes.

So then I wonder, maybe since this lib is fairly narrowly aimed at being minimal, we should take more time pondering the first case (the harder one, requiring the lovely parsing setup you've defined in this PR), and just jump on the second (freebie) case since it's an easy win.

Thoughts?

[lukeed]

@developit Should rely on browser built-ins as much as possible. They're actually pretty good.

We can get both points taken care of by just utilizing node.href, as you've discovered.

// starting at: http://example.com/blog

route('foo') //=> http://example.com/blog/foo

route('/foo') //=> http://example.com/foo

<a href="foo">foo</a> //=> http://example.com/blog/foo

<a href="/foo">foo</a> //=> http://example.com/foo

Doing it this way works with/without JS enabled. It also works regardless of what <Router/> context you are within, since it's all native-URL handling, which then leaves the Router to respond to changes.

[developit]

Right right - my point was that with route() we have to construct a link and run the URL through it. With the handling for existing links already in the DOM, those URLs are already parsed for us.

[ashsearle]

Reimplemented in #243