State storage in router location vs. Redux store

[dandelany]

Hi all - thanks for all the great work lately on 4.x.x. Apologies for opening another "existential discussion" ticket but I wanted to ask a few questions about the react-router-redux state model. I'm working on my first Redux app after several projects in Reflux, so some of this may just be some misunderstanding of how Redux is supposed to work.

Namely I want to question the axiom that "You should not read the location state directly from the Redux store." I know that it's currently unsupported due to react-router's asynchronicity, but I guess I'm wondering if there's a way to make it work. My use case is something like this:

I have a "page" in my SPA that is an interactive chart with a bunch of settings (filters, time ranges, that kind of thing). I'd like to store them in app state because some of the settings may be used by other components on other pages. So I wire the thing up with an UPDATE_CHART_SETTINGS Redux action and a chartReducer that maps the settings into state.chart.settings and it all works like a charm.

The thing is, I know that in a few weeks, the boss will come tell me that "settings X, Y and Z are the most important, we need to save them in the query string so that people can share links and see the same thing." So, (if I understand the docs correctly,) at that point I will need to add a history.pushState somewhere, either in the component onChange callback or (preferably?) in my action, to push the relevant settings to the URL when they change - so far so good. But I will also need to change how this state gets propagated to my components - I have to change my chart container's mapStateToProps to use props instead of Redux state, and I also have to thread react-router's location prop down to it, since it's not a top level Route in this case. And I have to do this for any other components which access this setting.

This seems like a code smell to me - fundamentally, the settings are all still the same kind of app state, and it feels strange to have to handle them differently based on which ones are "saved" in the URL. I think I would be mostly happy if react-router-redux could provide me with a way to know when it was safe to use the state stored in state.routing.location (ie. when the component tree matched state) - maybe something like a boolean state.routing.transitioning that, when true [edit: oops, meant false], guaranteed they were in sync.

But to go a bit further - the above solution would still require moving the Single Source of Truth about some of the settings out of state.chart.settings and into state.routing.location.query - it would be even better if I could just leave them where they were! Again, "URL-savedness" shouldn't dictate where the setting state is stored, IMO. I think in my perfect world, I would be able to define two reducer-esque functions somewhere that looked something like:

const querySettingKeys = ['filters', 'range'];
const paramSettingKeys = ['id'];

export function mapLocationToState(location, state) {
  switch(location.pathname) {
    case "/chart":
      const settings = Object.assign({}, state.chart.settings, 
        _.pick(location.query, querySettingKeys),
        _.pick(location.params, paramSettingKeys)
      );

      return _.merge({}, state, {chart: {settings}});

    default: 
      return state;
  }
}

export function mapStateToLocation(state, location) {
  const query = _.pick(state.chart.settings, querySettingKeys);
  const params = _.pick(state.chart.settings, paramSettingKeys);

  return _.merge({}, location, {query, params});
}

...and then wire them up to react-router-redux to keep them in sync. Something along these lines seems to me like it would a good functional approach to passing state between location and store - and it would be easyish to unit test by ensuring that mapStateToLocation(mapLocationToState(someLocation)) === someLocation. It would be great to at least get some feedback to know if this falls into the category of A) hmm, interesting... B) trivially possible already somehow, or C) crazy ramblings of a naive Redux dev who has not yet learned the Way. :)

Thanks very much for your time!

[erosenberg]

I agree! 👍

[billyjanitsch]

I realize that react-router-redux doesn't and has never done this, but I'm similarly wondering if it's possible.

When I think about my ideal redux + react-router integration library, all I really want is something that syncs the current route with some part of the Redux store. As @dandelany said, this would make it easy to move "page settings" in and out of the URL, depending on my serialization needs. Conceptually, this seems like "not a big change", but in the current model, it requires a lot of code reorganization.

In my mind, a huge advantage of using redux to manage state is that react-redux allows you to circumvent the awkward choice of passing props far down the tree or reaching into context. It seems unfortunate that I need to make this tradeoff if I want to use URL state in my selectors.

[aaronjensen]

I would like something like this as well. Ideally, the store would contain the location as well as the route params, but maybe that is too much to ask 😄

Our particular scenario is that we have a search/filter page that uses the query string to store state, similar to @dandelany's expressed concern. We maintain that query string through all navigation through the site so that you can always go back to your original search, even through a refresh.

To facilitate this, we have action creators like:

export const changeQuery = (searchParams) => (dispatch, getState) => {
  const pathname = getState().routing.location.pathname
  const queryParams = serializeSearchParams(searchParams)
  return dispatch(replace(pathname + queryParams))
}

export const changePath = (path, searchParams, forcePush) => (dispatch, getState) => {
  const state = getState()
  const currentPathname = state.routing.location.pathname
  const pathname = path ? getPathname(path) : currentPathname
  if (pathname === currentPathname && searchParams && !forcePush) {
    return dispatch(changeQuery(searchParams))
  }
  const queryParams = searchParams ?
    serializeSearchParams(searchParams) : getCurrentQueryString(getState())
  return dispatch(push(pathname + queryParams))
}

The first is for changing the query string w/o changing the current path, the second is for changing the path while keeping the query string the same (or changing it).

Now, it is probably safe for us to use locationBeforeTransitions instead of location in these cases because we only use the action creators as a result of user interaction. But, it is recommended against using locationBeforeTransitions. The alternative for us would be to pass location all the way down, or use a component to throw it in context (we use react-router) and pass location into the action creator. It's a lot of handing stuff around that really feels like it should be easier.

Am I missing something fundamental here?

[timdorr]

Sorry for the hold up. I've got a Wall of Text™ to read through here and I do want to give a meaningful response. I haven't had time to do that yet, but I'll be sure to do so in the next couple days.

[timdorr]

@dandelany I may be tuning this response a bit too much to your particular example, so I apologize if this isn't generally applicable:

It seems to me like this can be easily handled with some React lifecycle method usage. I don't know why, but they seem to have fallen out of favor lately and people forget they exist or think they're bad or something. Use them!

You should be keeping your chart component's state in your store. The URL is a reflection of that state, but it is not a source of truth. Instead, it can be used at key times: When the component is loaded and when the state changes? And there are two matching lifecycle methods just for this: componentWillMount and componentWillReceiveProps. Here's some pseudocode to do that:

componentWillMount() {
  if (this.props.chart_settings === null)
    this.props.restoreChartSettings(this.props.location);
}

componentWillReceiveProps(nextProps) {
  if (this.props.chart_settings !== nextProps.chart_settings)
    this.context.router.replace(serializeChartSettings(nextProps.chart_settings))
    // You should probably replace here so that they don't undo their changes if they go back
}

You don't need to inject URL serialization into your actions, just let them pick up on lifecycle events and build the behavior declaratively. React has a great lifecycle system that makes this stuff easy to implement without having to tack on yet another library and increase your "JS fatigue".

I'm also seeing a lot of folks trying to turn their store into some sort of event bus. This is not the function of the store. You should be using it simply as a state container and think about state changes more reactively, rather than as imperative event handlers.

@billyjanitsch The biggest problem with trying to force the router state into Redux is that it's not serializable. It's also relative to where you are in the route tree, so it's not absolute. this.props.route will be different for your root route than it is for a leaf node. So, even if we wanted to head in that direction, storage just isn't possible without reimplementing a lot of React Router code ourselves.

@aaronjensen I would make a similar recommendation for you to what I made above: use React lifecycle methods. You have a component that manages the search UI and triggers those changes to state. It can connect to that state and serialize the URL as it changes (your changeQuery action inside of cWRP).

I believe that answers most of the questions here. I'll close this since I don't think there's anything actionable here, but feel free to keep replying and I promise I'll keep up the conversation. And if anything comes up, we can open up a specific issue with specific actionable things to implement.

[dandelany]

Thanks for the detailed response @timdorr. Certainly not averse to using lifecycle methods, just wasn't sure of the intended usage here. I guess the key point here is that I have been thinking of routing/syncing location with state as mostly an app-level concern which should be handled outside of components, whereas you are saying the components should be in charge of firing actions which update the store from location when they mount. I don't know yet if I agree 100%, but your pseudocode makes sense and is cleaner than my current approach, so I'll try it this way.

The biggest problem with trying to force the router state into Redux is that it's not serializable.

But surely location is, right? I think the main idea here is just to have a way to automatically map the router location descriptor to a certain part of Redux state so the components don't have to do so when they mount.

[mjrussell]

@timdorr

You should be keeping your chart component's state in your store. The URL is a reflection of that state, but it is not a source of truth.

This actually surprised me quite a bit, I've been recently writing apps with much more URL linked behavior (filters, deep links, etc) and have come across the very challenges described in this issue. When I work with redux, one of my primary goals is to avoid duplicate state, and it seems like the URL is the single source of truth for this data. My approach to this problem has been to use reselect selectors which take ownProps and grab the data from the url bar. Updates to the url are done via action creators push and replace. This way my connected-component doesn't really have any idea the state comes from the URL or from the redux-store.

I don't love how splitting the url state into being in the store but also updated via the url bar would force logic for keeping this in sync to be included in every component. Maybe there's a way to write a HOC wrapper which could perform this synchronization? react-router-redux-param or something?

I'd imagine an interface like:

// React-Router Param Id Synching
const todoParamSelector = state => state.todo.selectedTodo
class TodoDetails extends Component {
   ...
}

RouterReduxParam(todoParamSelector)(TodoDetails);

// React-Router Query Param Synching
const todoFilterSelector = state => state.todo.todoFilter

RouterReduxQueryParam(todoFilterSelector)(TodoDetails);

[timdorr]

@dandelany

I guess the key point here is that I have been thinking of routing/syncing location with state as mostly an app-level concern which should be handled outside of components, whereas you are saying the components should be in charge of firing actions which update the store from location when they mount.

You can certainly just listen to your history instance, but it becomes harder to isolate that listening to just when you have your chart component available. Instead, if you use the lifecycle methods of the component, you can instead tie that behavior to when it is in use.

I think the main idea here is just to have a way to automatically map the router location descriptor to a certain part of Redux state so the components don't have to do so when they mount.

Yes, and it is under state.routing. However, this is the location before the router has had a chance to read it and render the appropriate routes. If you have async routes, then there may be a significant time between the location changing and the router actually updating. That's why we don't recommend reading the location from that source. Instead, you should get it from the route component params, as that will always be after the router has fully resolved all async behaviors. Plus, you don't have to map in any props manually; they are already provided.

@mjrussell

This is somewhat anecdotal, but the 4.0.0 version of this library maintains that the store is the source of truth for the location. We enhance the history instance to first put the location in the store and second notify listeners based on that updated state.

I would find it more troublesome to maintain two sources of truth. It changes the predictability of your state container's behavior. Also, you actually do care where the state came from, as mutating it via an action creator wouldn't work as expected. In fact, if you have parts you don't store in the URL, then you both have to do a replace/push and dispatch an action. That seems pretty hard to reason about and might trip up others working on the same app. If you make it more declarative, then no one has to concern themselves with storage at all.

You can certainly do a HoC method to mix in the location storage, but I don't think it's really needed. You only need two small lifecycle methods and you've got syncing implemented.

[mjrussell]

@timdorr

We enhance the history instance to first put the location in the store and second notify listeners based on that updated state.

Wow I missed that in the PRs, thats actually pretty nifty.

I would find it more troublesome to maintain two sources of truth. It changes the predictability of your state container's behavior.

Hmm we may be talking past each other here. I thought your proposed solution was to "sync" the router param/query with the url bidirectionally via lifecycle methods. This doesn't seem like a "single source of truth" to me because Im not sure which to believe when they differ.

Also, you actually do care where the state came from, as mutating it via an action creator wouldn't work as expected.

Thats fair, the action creator would know where the data lives (url vs state) but the component would just dispatch an action without that knowledge.

Thanks for the reply, will have me thinking about how I want to the url and store to work together going forward.

[dandelany]

I agree with @mjrussell, I'm still having a hard time resolving the "source of truth" question here. In your example, when the component mounts, it calls the restoreChartSettings action creator to update the store based on the router's location. In other words, the URL state drives the store state in this case, it's not just a reflection of state. So I don't think it's always true that "the store is the source of truth" - nor should it be, necessarily. On the other hand, in your componentWillReceiveProps the store state does determine the state of the URL (via router.replace). So it seems to me that there is already a "two sources of truth" problem.

After trying it, it seems like there are a few practical problems with doing it this way. The first is that, contrary to the comment in your example, I do want to use router.push to save state history in the browser history, so users can undo by clicking "Back". This doesn't work because, although history navigation (popstate) gives us a new location prop from the router, the store is driving the location state during componentWillReceiveProps, not vice versa. The other problem is: if I set some settings, then navigate to another "page" route which doesn't sync the URL (ie. click a <Link>), then come back to my chart page, my settings will be cleared. Even though they were saved in the store state, the (now empty) URL state overwrites the store state, because URL drives store state during componentDidMount. (even if it merged instead of overwriting, they'd get out of sync)

So to summarize, here is how I understand the state flow in your example. In the left column are events during which URL state determines store state (ie. URL is source of truth), vice versa on right.

URL -> Store	Store -> URL
componentDidMount	componentWillReceiveProps
	history navigation (fwd/back)
	action creator modifies state

Despite my initial gut feeling, I have come to accept that the answer to the question "who controls state, URL or store?" must necessarily be "a little of both" - because we obviously want to respond to history navigation, but we also don't want to have to use router.push/replace as a primary means of state mutation. But I think the ideal state flow is something more like:

URL -> Store	Store -> URL
Initial app load	componentDidMount
history navigation (fwd/back)	componentWillReceiveProps
	action creator modifies state

I'm not sure there is a good solution that lives entirely in component lifecycle methods as you propose, and is also capable of distinguishing between the initial app load and componentDidMount, which is leading me back to considering an app-level solution.

I realize that all of this may be well out of scope for what you want to do with react-router-redux but your advice is nonetheless very helpful if I end up implementing a different solution.

[dandelany]

Sorry for the double post, I wanted to address this as a separate but related issue @timdorr :

Yes, and it is under state.routing

By "certain part of Redux state," I mean a part that I specify, so that I do not have to move my source of truth to accommodate saving it in the URL. [edit to add: maybe I can already do this with selectLocationState and routeReducer?]

However, this is the location before the router has had a chance to read it and render the appropriate routes. If you have async routes, then there may be a significant time between the location changing and the router actually updating. That's why we don't recommend reading the location from that source. Instead, you should get it from the route component params, as that will always be after the router has fully resolved all async behaviors.

I am starting to wonder if we are talking about two separate use cases with two different solutions. What you are saying makes total sense to me for "location-y" things (scientific term) like rendering navigation - I wouldn't want the nav to update before the next route transitions in. And it also makes sense to use props.location for this kind of thing.

But I'm talking about using the URL to represent pieces of restorable core app state; data that is tied directly to something in the store, and is largely independent of the routes/components that are rendered. I think that in my use case above, if I were to use state.routing, not only is it OK that the value is updated before the router is done, it's desired. This is state that directly controls the <input>s which allow the settings to be changed, so if it does not update immediately, there will be a lag between changing the input and seeing the result.

This is a common pattern for me, and it would be great to have a way to add this "URL-saveability" superpower to any given part of state in the store, with minimal configuration and without changing how state is structured.

Thanks again for your time and advice! 🙏 I'd be happy to create an example repo demonstrating this stuff if any of it is unclear - let me know.

PS: Since this is an active discussion that seems to resonate with at least a few other members of the community, what do you think about re-opening this ticket for visibility and maybe giving it a discussion tag? Conversations in closed tickets are really hard for people to find.

[troch]

Interesting discussion. I agree with what is being said about URLs : they are not another source of truth of a redux / react app. They are like the DOM: a state change might update the DOM as a side-effect, and events can arise from DOM interactions to demand state changes.

Replacing with URL: a state change might update the URL as a side-effect, and events can arise from URL interactions (manual change, back & forward buttons) to demand state changes.

[bs85]

@dandelany @erosenberg @aaronjensen @troch @billyjanitsch @rpechayr @cimi

I've been playing with react-router for the first time, and your post really hit home. I wanted to implement the same split source of truth depending on the event (user navigation or state change), and I after a lot of going in circle I also tried to implement mapStateToQuery and mapQueryToState.

I spent a couple days on this to try to find a way, and I think I'm finally onto something. I haven't tested it extensively, but with my few pages it works perfectly under all circumstances. It is not idiomatic at all, but fairly straight forward and painless.

To avoid having to match location.pathname to route-specific logic manually, I created a small wrapper around react-router match(). This way I can put my utility functions directly on the routes. match() looks async, but as long as you don't use async routes [1] it returns within the same frame.

[1] https://github.com/ReactTraining/react-router/blob/master/docs/guides/DynamicRouting.md

Here's the wrapper to get a route based on a path string

getRouteFromPath(routes, path) {
    let route;
    let beacon;

    function extractRoute(_error, redirectLocation, props) {
        // recursively call extractRoute in case we get redirected
        if (redirectLocation) {
            match({ routes, location: redirectLocation }, extractRoute);
            return;
        }

        beacon = true;
        route = props.routes[props.routes.length - 1];
    }

    match({ routes, location: { pathname: path } }, extractRoute);

    // if beacon isn't set to true, extractRoute hasn't been run synchronously
    // and we're in trouble
    if (!beacon) {
        throw new Error('Router.match() is running async for some reason :/');
    }

    return route;
}

The routes and their mapping methods

const routes = {
    path: '/',
    component: App,
    indexRoute: { onEnter: (nextState, replace) => replace('/search') },
    childRoutes: [{
        path: '/search',
        component: Search,
        mapStateToQuery(state) {
            return Search.getParams(state).toJS();
        },
        mapQueryToState(dispatch, query) {
            return dispatch(Actions.search.setParams(
                _.pick(query, ['type', 'date'])
            ));
        },
    }, {
        path: '/pick',
        component: Pick,
        mapStateToQuery(state) {
            return Search.getParams(state).toJS();
        },
        mapQueryToState(dispatch, query) {
            return dispatch(Actions.search.setParams(
                _.pick(query, ['type', 'date'])
            ));
        },
        validatePropsOrRedirect(state) {
            if (
                Search.getParams(state).get('type') &&
                Search.getParams(state).get('date')
            ) {
                return null;
            }
            return '/search';
        },
    }],
};

In the main app file, I listen to history changes and "monkey-patch" the location. This is slightly savage, especially with the whole app built with FP in mind, but heh it works. Maybe a future version of react-router could implement a way to send a new location in the onBeforeChange listener.

The main app

const store = StoreFactory();

// filter empty parameters from the query to avoid ugly ?param=&param2=
const filterEmpty = map => _.pickBy(map, value => (
    value !== '' && value !== undefined && value !== null
));

function onBeforeHistoryChange(location) {
    // abusing the fact that we can mutate location in here to manipulate the history
    // for POP events, we try to update the state using the query
    // for other events, we use the current state to complete the query, so we don't
    // have to explicitly specify parameters everytime we navigate

    // ignore REPLACE events from the store middleware
    if (
        location.action === 'REPLACE' &&
        location.state && location.state.generated_by_store
    ) {
        return;
    }

    // get the actual route based on location.pathname, as state/query manipulation
    // function are defined directly on the route. Will break for async routes, god
    // forbid we need them at some point
    const route = RouterUtils.getRouteFromPath(routes, location.pathname);
    if (!route) return;

    if (location.action === 'POP') {
        // mapQueryToState dispatches regular actions
        if (route.mapQueryToState) {
            route.mapQueryToState(store.dispatch, location.query);
        }
    } else {
        if (route.validatePropsOrRedirect) {
            const redirectTo = route.validatePropsOrRedirect(store.getState());
            if (redirectTo) {
                location.pathname = redirectTo;
                return;
            }
        }

        // set the route query based on the current state
        if (route.mapStateToQuery) {
            const query = filterEmpty(route.mapStateToQuery(store.getState()));
            const newLocation = browserHistory.createLocation(
                _.extend({}, location, { query })
            );
            location.query = newLocation.query;
            location.search = newLocation.search;
        }
    }
}

// on the first page load, update the state based on the query
const route = RouterUtils.getRouteFromPath(routes, location.pathname);
if (route && route.mapQueryToState) {
    route.mapQueryToState(store.dispatch, browserHistory.getCurrentLocation().query);
}

browserHistory.listenBefore(onBeforeHistoryChange);

In the store, this middleware keeps the query in sync with the state

const setHistoryQueryMiddleware = store => next => action => {
    const result = next(action);

    const location = browserHistory.getCurrentLocation();
    const route = RouterUtils.getRouteFromPath(routes, location.pathname);
    if (!route) return result;

    if (route.mapStateToQuery) {
        const query = filterEmpty(route.mapStateToQuery(store.getState()));
        // history.replace needs to run AFTER the store is updated, otherwise
        // our onBeforeUpdate listener is going to use the previous version of
        // the store to set the route params
        window.setTimeout(() => {
            // generated_by_store allows onBeforeChange to ignore those changes
            // as they do not need further processing, and can cause loops
            browserHistory.replace(
                _.extend({}, location, { query, state: { generated_by_store: true } })
            );
        });
    }

    return result;
};

[benjamine]

If you have async routes, ...

I suppose many of us choose to avoid async routes at all, it would be nice to have a solution for us, even if it were behind a config flag.