Router.run Handler not idempotent

[matthewwithanm]

I'm using an approach similar to many of the examples—hanging a static method off of Handlers, accumulating them in the Router.run callback, and rendering the app. However, instead of these methods returning data, they mutate an application state store. Changes to the application state should also trigger a new render. It looks something like this:

let CurrentHandler;

function renderApp(Handler) {
  CurrentHandler ||= Handler;
  if (!CurrentHandler) return;

  React.withContext({appState}, () => {
    React.render(<CurrentHandler />, container);
  });
}

// Rerender the screen whenever the URL changes.
Router.run(routes, Router.HistoryLocation, (Handler, routerState) => {
  (async function run() {
    await runStaticMethods(routerState.routes, routerState, actions);
    renderApp(Handler);
  }());
});

// Rerender the screen whenever the app state changes.
appState.on('change', () => renderApp());

The problem is that Handler is actually the (stateful) router. That means that (if a change event is dispatched on the appState during the course of running the static methods), the rerender it triggers will be with the new screen. (Expected behavior is that it will continue to render the old screen until the static methods complete.) As a result, the new screen can be rendered before its appState dependencies are met.

This is the first earnest look at react-router I've done since the new API a few months back, so I'd appreciate advice on how this should be accomplished, but I also think it would be great for the Handler rendering to be idempotent so that rendering could happen outside of the run callback.

Apologies if I'm misunderstanding something. Thanks!

[agundermann]

If I'm understanding you correctly, this is related to #674 .

So perhaps this would work:

var lastState = null;
Router.run(routes, Router.HistoryLocation, (Handler, routerState) => {
  (async function run() {
    lastState = routerState;
    await runStaticMethods(routerState.routes, routerState, actions);
    if(lastState === routerState)
      renderApp(Handler);
  }());
});

Also, when rendering async, beware of this issue: #1027

I also think it would be great for the Handler rendering to be idempotent so that rendering could happen outside of the run callback.

I agree. That's what we're aiming for (#1012).

[matthewwithanm]

So perhaps this would work:

Hm, that's not going to do it; the render we have to guard against is the one triggered by the "change" event. I can just remove the listener before the static methods and add it back afterwards but it seems like a hack.

I agree. That's what we're aiming for (#1012).

👍

[agundermann]

Ah, I overlooked that event.

In that case, you could also try to work around rerendering Handler, e.g. by causing a setState on your top level handler instead, since Handler.willReceiveProps is where the state change happens.

[ryanflorence]

follow #1031