Upgrade React Router to v4

[fpagnoux]

There was several bugs recently (e.g. #202 #199 ) that we suspect were related to React Router.

Upgrading it needs to be done at some point anyway, and could save us trouble.

[Morendil]

I advise against for the reasons in #202 - it's entirely possible we'd only be changing who gets a head start in a race condition, leading to the dreaded "it works on my machine" effect. At the moment it repro's reliably on one machine (mine, at least this morning), I'll investigate on my spare time as this is a production bug.

[Morendil]

Also worth noting: "React Router v4 is a complete rewrite, so there is not a simple migration path."

It might save us trouble, but it will also definitely cause some.

[bonjourmauko]

Hi ! I think this issue is more a theme, a regroupment of several issues. It needs:

• investigation
• eventually feasibility testing
• and slicing

There are several things we know we have to change :

• changes to the history API
• introduction of specialised history constructs
• deprecation of nested routes
• migration from monolith routes to component-based ones

And many other we do not know yet.

It is highly advisable to investigate and prototype up-front before investing a lot of time on this.

[sandcha]

A good react-router migration guide is available here.

After some feasibility testing on to-react-router-v4 branch, imho, the main issue is the lack of appropriate tests (on history for example) to understand how far we break the app at a migration step.

[Morendil]

The above guide is missing some details, for instance locationShape and routerShape are no longer available.

[fpagnoux]

Making this issue a "theme" feels a little much to me. If upgrading the router is such a big task that it's worth becoming a theme, then maybe it's just not worth doing it.

IMHO, a reasonable way to proceed if we are unsure about the complexity would be to time-box a fixed amount of time and see how it looks (what @sandcha seems to be doing 👍).

I'll also point out that the legislation explorer is a relatively simple app, with 4 types of pages, 3 of them just displaying static content. So I'd not be scared beyond measure about breaking something so subtle that we would not be able to identify it with some relatively simple tests.

[fpagnoux]

At the moment it repro's reliably on one machine (mine, at least this morning), I'll investigate on my spare time as this is a production bug.

If we can reliably reproduce the current bugs, then yay let's go for it and fix these before thinking about the upgrade.

[Morendil]

Let's deal with more urgent updates first: https://github.com/openfisca/legislation-explorer/network/alerts