Server side doesn't receive correct location after match()

[psalz]

The new applyRouterMiddleware approach to things causes an issue where the server side code doesn't receive the correct routing information after a call to match. This is actually happening in the todo-example, where requesting anything other than / (e.g. /completed) will cause React to complain about invalid checksums. When disabling client-side Javascript, you will see that only the / route is rendered, no matter the actually requested route.

I'm anything but involved in the react-router codebase, but I think I have found the root cause and also have a potential solution, although I'm not sure if this should be resolved in userland (here) or in react-router itself.

Previously, when rendering a RouterContext, the location prop was directly provided by the renderProps returned from match. With the new approach, this is no longer the case: The Router component uses its state for the location. This state variable is changed in a history listener, here. Note that this listener immediately fires once upon registering, so this is not an async issue. The problem lies a couple of lines below: As this newly created history doesn't yet have a location, a call to getCurrentLocation is made. Assuming a MemoryHistory, which is the default, the call ends here, where entries[current] simply equals / at this point (see line 36). This never gets changed, so the Router state is never updated, and all child components receive / as the location.

I'm wondering if the TransitionManager should update the history state upon a successful match.

Anyway, in the meantime, simply adding

renderProps.router.replace(renderProps.location)

to prepareData solves the issue for me.

Note that applying this fix may still cause invalid checksums at times, but this is due to another issue (facebook/react#6451).

[denvned]

@psalz Thank you very much for the investigation and for the workaround!

[psalz]

Thanks for the quick response!

[denvned]

I've also submitted a proper fix for this issue to react-router repo: remix-run/react-router#3394

[mattecapu]

Great job @psalz, I was experiencing the same issue but I wasn't able to find the cause.
And as always thanks to @denvned for his dedication to the project 😄

[taion]

I'm not sure where you're seeing getCurrentLocation being called. The server rendering path should have no dependence on the current location. You have access to the router props as the 2nd arg to creating the router context in the middleware, and you should make use of those as appropriate.

[denvned]

I'm not sure where you're seeing getCurrentLocation being called.

@taion Note, with isomorphic-relay-router we are using Router instead of RouterContext for both, client side and server side rendering. That allows to keep many things simpler. And this issue is the only known problem with that approach so far.

[taion]

You should not use <Router> on the server. We're trying to move in a direction where we don't have a history on the server at all – in this case we're just using the history for convenience for generating hrefs and paths.

[denvned]

We're trying to move in a direction where we don't have a history on the server at all

@taion, Are you sure this is the right direction? Having the history on the server allows unification of server side and client side rendering. Using Router on the server and client allowed the implementation of isomorphic-relay-router to be much more simple.

[taion]

There's definitely code that's shared between client and server, and we want to have a simple API for extending things – but I don't think stateful operations like getCurrentLocation, push, or replace should be supported when running on the server, in a default mode. The use of createMemoryHistory as the default there is not strictly correct – possibly in that case we should just strip out the unused methods.

One downside with <Router> is that without also using match, you can't properly render for code-splitted routes on the server anyway.

[denvned]

One downside with <Router> is that without also using match, you can't properly render for code-splitted routes on the server anyway.

@taion, I don't mind to use match with Router at all. Actually, in isomorphic-relay-router I have to use match during both, server side, and client side rendering. I rather mind to be forced use RouterContext instead of Router on the server, because it complicates implementation of this package.