Hook an external listener

[zalmoxisus]

In order to link side effects with actions, we're introducing a new option listener (I'm not good at naming, so any other suggestions are welcome).

Here's how it would work for history (assuming that the location is changed with CHANGE_LOCATION action, otherwise we could compare, for example, an action payload key):

let historyIdx = 0;
let historyIndexes = [historyIdx];

const listener = (liftedState, liftedAction) => {
  if (liftedAction.type === 'PERFORM_ACTION') {
    if (liftedAction.action.type === 'CHANGE_LOCATION') historyIdx++;
    // or: if (liftedAction.action.router !== undefined) historyIdx++;
    historyIndexes.push(historyIdx);
    return;
  }

  if (liftedAction.type === 'JUMP_TO_ACTION') { // time travelling
    history.go(historyIndexes[liftedAction.actionId] - historyIdx);
  }
  else if (liftedAction.type === 'JUMP_TO_STATE') { // legacy - we'll deprecate this action
    const id = liftedState.stagedActionIds[liftedAction.index];
    if (historyIndexes[id]) history.go(historyIndexes[id]);
  }

  // Replace the location if the corresponding state changed during hmr, persisting, importing...
  const currentState = liftedState.computedStates[liftedState.currentStateIndex];
  const locationInStore = currentState.router.location.pathname;
  if (history.location.pathname !== locationInStore) {
    history.replace(locationInStore);
  }
}

const store = createStore(rootReducer, window.__REDUX_DEVTOOLS_EXTENSION__({ listener }));
// or const store = createStore(rootReducer, DevTools.instrument({ listener }));

Instead of accumulated historyIndexes we could generate them from liftedState.actionsById, but it's more declarative and, anyway, it's intended only for development or when window.__REDUX_DEVTOOLS_EXTENSION__ is defined.

We could also use it for other cases when need to link side effects, as, for example, for redux saga devtools.

\cc @mjackson @ryanflorence @supasate @timdorr @taion @gaearon

[taion]

Is the idea here that developers can provide listeners to users for cases where custom integration with Redux dev tools is required?

[zalmoxisus]

@taion exactly. It can be composable and used like so:

import { reduxDevToolsListener } from 'react-router-redux';

const store = createStore(rootReducer, DevTools.instrument({
  listener: reduxDevToolsListener
}));

Or we could add an additional argument:

const store = createStore(rootReducer, DevTools.instrument({ /* options */ }, listener));

[zalmoxisus]

Regarding react-router, there's also connected-react-router. @supasate did a great job, but it has 2 issues for now:

• we don't want extra operations when not needed
• we don't want to push a new url in the history when time travelling.

Basically, we could make a "hackable" middleware, which will do the same as my listener example:

const routerMiddleware = (history) => {
  let historyIdx = 0;
  let historyIndexes = [historyIdx];

  return store => next => action => {
    setTimeout(() => { // the middleware is called before devtools enhancer, so will wait for the next tick
      if (store.liftedStore) { // is `redux-devtools-instrument` applied
        const liftedState = store.liftedStore.getState();
        const currentId = liftedState.nextActionId - 1;
        if (!historyIndexes[currentId]) { // is 'PERFORM_ACTION'
          if (action.type === 'CHANGE_LOCATION') historyIdx++;
          historyIndexes.push(historyIdx);
          return;
        }

        if (liftedState.currentStateIndex !== liftedState.stagedActionIds.length - 1) {
          // time travelling
          history.go(
            historyIndexes[liftedState.stagedActionIds[liftedState.currentStateIndex]] - historyIdx
          );
        }

        // Replace the location if the corresponding state changed during hmr, toggling actions...
        const currentState = liftedState.computedStates[liftedState.currentStateIndex];
        const locationInStore = currentState.router.location.pathname;
        if (history.location.pathname !== locationInStore) {
          history.replace(locationInStore);
        }
      }
    });
    return next(action)
  }
}

The only issue here is IMPORT_STATE. In case of listeners, we have the liftedAction and can address it.

[supasate]

Sounds very good! I don't know about liftedState and liftedAction yet but I'll check them out.

About time travelling, at history.go(n), we might need to subtract the target position with the current position because n is the relative position to the current history position (in the browser history stack) (ref).

[taion]

Eh, sounds good if it's useful.

FWIW I think pretty fundamentally that this isn't the right approach for integrating routing with Redux, and that you'd be better off if you followed either:

• React Navigation's pattern of updating the URL as a side effect of navigation that primarily takes place within Redux, or
• Farce's approach of making the Redux store the full state of truth for the current location

But whatever – this seems generic enough.

[zalmoxisus]

@supasate, yes, we can use history.go(historyIndexes[liftedAction.actionId] - historyIdx). I've updated the snippets.

@taion, this is a POC for @mjackson's tweet of what we can do from Redux DevTools part. For the extension we had also a way to check if a state update is the result of time travelling, hot reloading..., but it was even worse.

[taion]

Right, this is an adequate workaround for history's quirks.

Really the right way to handle this would be something like the way React Navigation deals with things per https://github.com/react-community/react-navigation/blob/v1.0.0-beta.5/website/src/BrowserAppContainer.js#L22-L39 (but perhaps in a store enhancer or subscriber), and then dev tools stuff just wouldn't be a special case any more.

[zalmoxisus]

I agree with @taion that doing it in a store enhancer or subscriber would is better. However, we still need a way to figure out whether a state was updated due to a client action or from a DevTools monitor (and if the latter we should handle that action accordingly). That aims to be solved in #16. So, the example above would become:

let historyIdx = 0;
let historyIndexes = [historyIdx];

if (store.liftedStore) { // subscribe only if devtools instrumentation applied
  store.subscribe(() => {
    const liftedState = store.liftedStore.getState();
    const liftedAction = store.liftedStore.getState().liftedAction;

    if (liftedAction.type === 'PERFORM_ACTION') {
      if (liftedAction.action.type === 'CHANGE_LOCATION') historyIdx++;
      // or: if (liftedAction.action.router !== undefined) historyIdx++;
      historyIndexes.push(historyIdx);
      return;
    }

    if (liftedAction.type === 'JUMP_TO_ACTION') { // time travelling
      history.go(historyIndexes[liftedAction.actionId] - historyIdx);
    }
    else if (liftedAction.type === 'JUMP_TO_STATE') { // legacy - we'll deprecate this action
      const id = liftedState.stagedActionIds[liftedAction.index];
      if (historyIndexes[id]) history.go(historyIndexes[id]);
    }

    // Replace the location if the corresponding state changed during hmr, persisting, importing...
    const currentState = liftedState.computedStates[liftedState.currentStateIndex];
    const locationInStore = currentState.router.location.pathname;
    if (history.location.pathname !== locationInStore) {
      history.replace(locationInStore);
    }
  });
}

It should work for a connected component like in remix-run/react-router#4717 just fine. In case of a store enhancer we should either check store.liftedStore inside store.subscribe or execute in the next tick, since the enhancer will be applied before devtools instrumentation. It will not work for middlewares, since they don't get liftedStore.

[zalmoxisus]

Closing in favour of #16.