Fix non-routing Link component usages (#210)

[denis-sokolov]

#210

[request-info]

The maintainers of this repository would appreciate it if you could provide more information.

[EmilTholin]

Will this work when using the hash version of the Router?

[denis-sokolov]

Good point, I haven’t tried it.

It should work, as the user of the components should still provide a proper, non-fragment URL, it is the responsibility of history/react-router to encode the output in whatever format they currently need.

[denis-sokolov]

I can try and verify it, maybe tomorrow.

[denis-sokolov]

No meaningful BC breakages, here are the findings:

1. <Router type="hash"> and regular links keep working. <Link to="about"> matches our isRoutingUrl check and thus gets passed through to react-router, which renders a <a href="#/about">.

2. <Router type="hash"> and links that manually have the hash route kind of work. <Link to="#/about"> does not match our isRoutingUrl check and gets rendered with the newly introduced shortcut into a <a href="#/about">. react-router handles this links, so it responds to user interaction and changes routes, but it does not have aria-current. We could attempt to distinguish to="#/about" as a non-fragment link, but technically ids can start with a slash. The usage <Link to="#/about"> is already kind of wrong, and with proper history support nearly universal, I don’t think we have to worry about this.

3. <Router type="hash"> and fragment links are still broken after the latest changes. <Link to="#subheading"> gets rendered with the newly introduced shortcut into a <a href="#subheading">, but is still handled by react-router to navigate to #/subheading, ignoring the fragment link. This was already broken with the react-router <Link>, and is still broken with our manually rendered <a>. Nothing to see here.

[EmilTholin]

The documentation for Link and NavLink should probably be updated as well to indicate that they are custom react-static components now, since react-router doesn't support hash links.

It might also cause some confusion that a hash NavLink will not have an active class or aria-current attribute added to it when it is active.

[denis-sokolov]

The documentation for Link and NavLink should probably be updated as well to indicate that they are custom react-static components now, since react-router doesn't support hash links.

If a package user decides that for hash links they need a manual <a>, good for them. They control their fragment links, I am happy for them. This change was meant to be invisible, to stop counter-intuitive behavior of a broken <Link>. It’s not meant to canonically replace all <a> in users’ applications. Thus, I don’t think it warrants a mention in the documentation.

It might also cause some confusion that a hash NavLink will not have an active class or aria-current attribute added to it when it is active.

Hypothetically, yes. But those are extra features that someone with an understanding of routing should expect, but then they should also expect them to not work. Still, you’re right, so I’ve added a warning in #232.