Giter Club home page Giter Club logo

Comments (8)

dictions avatar dictions commented on May 25, 2024

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);

from connected-react-router.

supasate avatar supasate commented on May 25, 2024

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

from connected-react-router.

dictions avatar dictions commented on May 25, 2024

@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.

from connected-react-router.

supasate avatar supasate commented on May 25, 2024

@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!

from connected-react-router.

dictions avatar dictions commented on May 25, 2024

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)
			}
			
		}
	}
}

from connected-react-router.

supasate avatar supasate commented on May 25, 2024

@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?

from connected-react-router.

dictions avatar dictions commented on May 25, 2024

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!

from connected-react-router.

supasate avatar supasate commented on May 25, 2024

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

from connected-react-router.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.