Time Travelling does not work

[supasate]

With Redux DevTools, everything seems to work well except time travelling.

[dictions]

Hey there, I think I can tackle this since it's blocking for me. Any insight as to why this might be?

Edit: If I would have looked closer I would have seen that you're reading directly from history 😉 Why don't we populate the store with the first history state, rather than firing the first action, then always render from the state of the store? That seems to fix time travel. (Forgive my tabs while I'm editing)

// reducer.js
import { LOCATION_CHANGE } from './actions'

/**
 * This reducer will update the state with the most recent location history
 * has transitioned to.
 */
export default history => {
	const initialState = {
		location: history.location,
		action: history.action
	};
	
	return (state = initialState, { type, payload } = {}) => {
		if (type === LOCATION_CHANGE) {
			return {
				...state,
				...payload,
			};
		}

		return state;
	};
};

// store.js
import {combineReducers} from 'redux';
import createRouter from 'connected-react-router';
import {createBrowserHistory} from 'history';

export const history = createBrowserHistory()

export default combineReducers({
	router: createRouter(history)
});

// ConnectedRouter.js
import React, {Component, PropTypes} from 'react';
import {connect} from 'react-redux';
import {StaticRouter} from 'react-router';
import {onLocationChanged} from './actions';

/*
 * ConnectedRouter listens to a history object passed from props.
 * When history is changed, it dispatches action to redux store.
 * Then, store will pass props to component to render.
 * This creates uni-directional flow from history->store->router->components.
 */
class ConnectedRouter extends Component {
	constructor(props) {
		super(props);
		
		this.unlisten = props.history.listen((location, action) => {
			props.onLocationChanged(location, action);
		});
	}
	
	componentWillUnmount() {
		this.unlisten();
	}

	render() {
		const {location, action, history, basename, children} = this.props;

		return (
			<StaticRouter
				action={action}
				location={location}
				basename={basename}
				onPush={history.push}
				onReplace={history.replace}
				blockTransitions={history.block}
			>
				{children}
			</StaticRouter>
		);
	}
}

ConnectedRouter.propTypes = {
	history: PropTypes.object.isRequired,
	location: PropTypes.oneOfType([ PropTypes.object, PropTypes.string ]),
	action: PropTypes.string,
	basename: PropTypes.string,
	children: PropTypes.oneOfType([ PropTypes.func, PropTypes.node ]),
	onLocationChanged: PropTypes.func.isRequired
};

const mapStateToProps = state => ({
	action: state.router.action,
	location: state.router.location
});

const mapDispatchToProps = dispatch => ({
	onLocationChanged: (location, action) => dispatch(onLocationChanged(location, action))
});

export default connect(mapStateToProps, mapDispatchToProps)(ConnectedRouter);

[supasate]

Nice catch!

ConnectedRouter.js should be changed like you proposed.

Can we just create a history instance in reducer.js like the below code instead of passing history object to it? So, it'll be easier for setting up the root reducer; especially, we we make rootReducer as a separate file.

import { createBrowserHistory } from 'history'
import { LOCATION_CHANGE } from './actions'

/**
 * This reducer will update the state with the most recent location history
 * has transitioned to.
 */
const history = createBrowserHistory()
const initialState = {
  location: history.location,
  action: history.action,
}

const routerReducer = (state = initialState, { type, payload } = {}) => {
  if (type === LOCATION_CHANGE) {
    return {
      ...state,
      ...payload,
    }
  }

  return state
}

export default routerReducer

[dictions]

@supasate I thought about that, but I have reservations because:

1. It becomes less testable because we're using createBrowserHistory which is dependent on window right?
2. We need to expose the history object somehow so that StaticRouter has access to it, right?

If we passed it in, it could be mocked in a testing environment, and we would give StaticRouter access to it for push, replace, etc.

[supasate]

@dictions I agree with testability.

The history object need to be passed to Provider, store, and rootReducer. If they are in separated files, the rootReducer might need to be transformed to a function that takes history object in.

However, I'd like make it simple by not changing the way most people do with the rootReducer file with the benefit of only initial state in the reducer.

I'll rethink it this weekend probably. If you have any idea, please let me know!

[dictions]

Yeah it's a tricky one. You either have to pass it to your rootReducer and make that a generator. Or, you'll need to create and colocate history with your rootReducer, making it not testable in the same way. What if the API was similar to redux connect?

// Your main client entrypoint

import rootReducer from 'reducers'
import configureStore from './store'
import {connectRouter} from 'connected-react-router'
import {createBrowserHistory} from 'history'

const history = createBrowserHistory()
const store = configureStore(
  connectRouter(history)(rootReducer)
)

// Set up router ...

Is that too ridiculous?

Edit: that might look something like this?

import { LOCATION_CHANGE } from './actions'

function routerReducer(state, { type, payload } = {}) {
	if (type === LOCATION_CHANGE) {
		return {
			...state,
			...payload,
		};
	}

	return state;
};


function connectRouter(history) {
	const initialRouterState = {
		location: history.location,
		action: history.action
	};
	
	return function wrapReducer(reducer) {
		
		return function(state, action) {
			const reducerResults(state, action)
			
			return {
				...reducerResults,
				router: routerReducer(state = initialRouterState, action)
			}
			
		}
	}
}

[supasate]

@dictions I think it's a nice solution. Based on your code, I fixed some bugs and Time Travel works great now (except URL bar is not updated which we may fix it as another issue).

import { LOCATION_CHANGE } from './actions'

/**
 * This reducer will update the state with the most recent location history
 * has transitioned to.
 */
const routerReducer = (state, { type, payload } = {}) => {
  if (type === LOCATION_CHANGE) {
    return {
      ...state,
      ...payload,
    }
  }

  return state
}

const connectRouter = (history) => {
  const initialRouterState = {
    location: history.location,
    action: history.action,
  }
  // Wrap a root reducer and return a new root reducer with router state
  return (rootReducer) => (state, action) => {
    let routerState = initialRouterState

    // Extract router state
    if (state) {
      const { router, ...rest} = state
      routerState = router || routerState
      state = rest
    }
    const reducerResults = rootReducer(state, action)

    return {
      ...reducerResults,
      router: routerReducer(routerState, action)
    }
  }
}

export default connectRouter

Would you like to submit a PR to be a contributor of this idea?

[dictions]

Looks great! I'll submit a PR in a bit.

I noticed that the URL bar was out of sync w/ the router reducer. I can't seem to figure out a solution to that though, as it would involve sending undo payloads to the browser history, and keeping track of the "real" browser history in memory.

My best example would be this:

1. Start on Page A
2. Nagivate to Page B
3. Go back in browser history to Page A (Page B is forward in history now).
4. "Undo" the navigate change via the Redux Devtools LogMonitor, thereby going forward to Page B.

I can't seem to wrap my head around a simple solution there. If you can figure it out I'd love to hear ideas.

PR coming soon!

[supasate]

About synchronizing URL in Time Travel mode, I've created a new issue #8 for this topic.