Transparently support <base href> in HTML documents

[mjackson]

Now that we have useBasename, it would be neat if we could transparently support HTML documents that use the <base> element to set a base URI for all relative links on a page. It could be part of createDOMHistory, or possibly its own history enhancer.

See remix-run/react-router#2014 for previous discussion.

[mjackson]

@MoOx Would you like to take a shot at a PR here?

[MoOx]

Hi @mjackson. Sorry but I have others priorities atm, I have used a workaround that satisfies my need. Maybe later but I cannot guarantee anything. Anyway, thanks for the work you made!

[loganfuller]

I'm running into an issue with the way that react-router automatically detects the base tag. I'm currently using <base target="_blank"> in an iframe to enforce that all links open in a new tab, but react-router currently attempts to grab the href value of the base tag (which defaults to "") and then sets basename to "/". Ideally react-router would detect that no base href has actually been set and ignore the tag.

[taion]

What do you mean "ignore the tag"?

Are you also passing in basename to createHistory?

[loganfuller]

@talon I'm just using the base tag to enforce the target of all links in the document, but it's having the side effect of causing react-router to consider all paths without a query string to be "/". I meant that the base tag would be ignored for the purpose of automatically setting a basename if there is no href attribute set.

I'm not passing a basename to createHistory. I'm using a vanilla browserHistory from react-router/lib/browserHistory.

[taion]

Sorry, I still don't understand what the problem is. What specifically are you seeing? There is no logic to automatically infer basename; if basename is not set, we should have the same behavior as if it were an empty string.

[loganfuller]

It's ok, I could be completely off base here - I haven't done much looking into the actual react-router codebase.

According to a9759df react-router automatically uses the href attribute of the base tag if one is found and no explicit basename has been set for the history. I'm assuming this is causing a side effect somewhere, since simply removing the base tag resolves the issue and the issue is not seen when rendering on the server.

document.location.pathname is showing the correct pathname of /flyers/embed/home-hardware-building-centre/1177291, while react-router is using a pathname of / when I sniff the location using history.listen and is rendering the wrong route. Removing the base tag causes react-router to render the proper route using a pathname of /flyers/embed/home-hardware-building-centre/1177291.

Strangely, simply appending a question mark to the URL seems to resolve the issue as well.

[taion]

I see. Thanks. Could you open this as a new issue? Sounds like a bug.

[taion]

Hit "comment" too quickly, rewriting this.

[taion]

I took a look. The current behavior is just plain wrong.

For an empty <base>, its href is the current URL. As noted, though, this is something that only applies to relative paths, not to absolute ones.

The real problem is that if we have something like <base href="/foo">, something like history.push('/bar') will take us to /foo/bar, but the actual href on a <a href="/bar"> will just be /bar (rather than /foo/bar), so we're misinterpreting <base href> here.

Unfortunately, we can't fix this with the current API. The expedient thing to do would be to update https://github.com/mjackson/history/blob/v2.0.1/modules/useBasename.js#L18 to use base.getAttribute(href) instead of extractPath(base.href), which would fix the use case here. It'd still fundamentally be incorrect, though.

[taion]

And in the router context, this qualifies as a silly obnoxious inconsistency. I think we should just get rid of <base> support; it doesn't do what we want here.

[taion]

One option is to use a custom attribute like <base basename="/foo">. Alternatively, we should consider dropping this feature altogether, given the difficulty of using it in a reasonable way with isomorphic rendering.

[taion]

@mjackson wdyt of using something like <base basename="/foo> for 3.x?

[mjackson]

@taion I don't see why we can't fix this with the current API. If <base href> doesn't apply to an absolute <a href>, we should be able to take that into account in createHref, right?

[taion]

Sorry, what I mean is – we can continue to support <base href>, but the meaning is different from what we do with basename.

If we have basename set to /foo, and do history.push('/bar'), we end up on /foo/bar, which I think is what's expected.

If we have <base href="/foo">, clicking on <a href="/bar"> will still take you to /bar.

In other words, our transparent support for <base href> as basename is actually contrary to what <base href> normally does.

[mjackson]

Ah, I see. Thanks for clarifying.

In that case it does seem like we should just drop support for <base href> as basename. However, I think this means <Link> will need to be a little smarter. Instead of pulling the pathname directly from this.props (as to) it should probably query the DOM node directly and let the browser resolve the pathname in case there is a <base> tag in the document.

[taion]

That's technically true but it might not matter much in practice.

In the case where <base href> is on a different origin, the HTML5 history API is just going to throw a SecurityError anyway, so it doesn't matter what we do (unless we support <Link>s that don't call into history, which would be a bit odd).

For relative links, I think it'd make the most sense for them to be relative to the route tree anyway, and otherwise ignore <base href>.

It might matter for relative transitions in history, e.g. for something like history.push('./bar'), but this is probably less of a priority.

I do think it might be nice to invent a <base basename>, though. The current router API makes it a bit messy to customize the supplied history. Then again, maybe not – this won't work on the server, for example.

[mjackson]

Ok, sounds like the best way forward is to deprecate and remove <base href> support for now. I'll cut a new minor 2.x release that deprecates it and remove it entirely in 3.x.

[taion]

We might want to fix the React Router examples then: https://github.com/reactjs/react-router/blob/v2.2.4/examples/active-links/index.html.

[taion]

This is going way off-topic, but what do you think of the idea of enhancers that can take an already-created history? Something like withBasename(history, basename)? Then the API with the React Router history singletons would be a little terser – instead of:

useRouterHistory(createBrowserHistory)({ basename })`

we could just do something like:

withBasename(browserHistory, basename)

or something along those lines. Same with useQueries.

[mjackson]

A function that accepts a createHistory function, rather than a history
object, is able to affect initialization of that object, so it's strictly
more powerful than the alternative. However, looking at the history
enhancers we have in the core library, they all just immediately create a
history object with the provided options anyway, so maybe it's not
necessary.

I like the withBasename example you gave. I think it's worthwhile to
explore other use cases as well.

On Mon, Apr 18, 2016 at 10:02 AM Jimmy Jia notifications@github.com wrote:

This is going way off-topic, but what do you think of the idea of
enhancers that can take an already-created history? Something like withBasename(history,
basename)? Then the API with the React Router history singletons would be
a little terser – instead of:

useRouterHistory(createBrowserHistory)({ basename })`

we could just do something like:

withBasename(browserHistory, basename)

or something along those lines. Same with useQueries.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#94 (comment)