Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use reducer-based paradigm for routing state #3190

Closed
taion opened this issue Mar 14, 2016 · 10 comments
Closed

Use reducer-based paradigm for routing state #3190

taion opened this issue Mar 14, 2016 · 10 comments

Comments

@taion
Copy link
Contributor

taion commented Mar 14, 2016

We should eventually move to a reducer-based paradigm for routing state, as with the new NavigationExperimental in React Native. This just seems to be the best paradigm for maintaining application-level state.

This gives us the potential for much better integration with Redux. The biggest challenge for the router, though, is that, given async routes, our state is not naturally serializable, which is not strictly a problem with a reducer-based paradigm, but would be a problem for Redux integration.

cc @gaearon @jlongster

@timdorr
Copy link
Member

timdorr commented Mar 14, 2016

The serializability problem definitely makes react-router-redux more difficult to completely integrate, but also having visibility into all of router's internal state doesn't seem to have any advantages as far as I can tell. In fact, it makes it really easy to mess with things in ways that produce bugs we wouldn't otherwise see.

What specific advantages does this afford us? It would seem we have more straightforward needs than a RN app with multi-dimensional navigation and more complex state. We simply map a URL to a particular route tree path. It's very roundabout and convoluted from a simple router, but it would seem we already have this, no?

@taion
Copy link
Contributor Author

taion commented Mar 14, 2016

It's just cleaner architecturally; it gives better hooks for other frameworks to integrate into the data lifecycle, and lets us split out route matching from state management.

@gaearon
Copy link
Contributor

gaearon commented Mar 14, 2016

Why is the state not serializable? I still don’t quite get it. For example, components don’t need to be in the state if they are given unique IDs that are deterministic given the same version of the code.

@taion
Copy link
Contributor Author

taion commented Mar 14, 2016

I was thinking earlier of doing something like that where you identify the current matching state with some set of indices of the matched routes, something like what React does – e.g. 0.1.3.0 or something.

This works okay in the synchronous case. I think this breaks down a bit in the async case – if getChildRoutes uses System.import, we'd have to build a local cache of routes, and also getChildRoutes need not be deterministic.

I haven't worked out all the cases, but the line of play is very tricky when we assume that we have async getChildRoutes handlers that may not always return the same value.

@taion
Copy link
Contributor Author

taion commented Mar 17, 2016

The main complexity here remains async routes – with potentially async getChildRoutes and getComponent, it won't be possible to synchronously resolve any serializable state structure into the actual route and component objects.

Maybe the serialized state should just include the location, plus a key into some cache that's separate from the store.

@jedwards1211
Copy link

jedwards1211 commented Jul 13, 2016

What is the best practice for working around unserializable state anyway? To me it is nearly inevitable, for instance if an app dynamically loads reducers for different routes, then the presence/absence of those reducers is philosophically part of app state, whether or not stored in the redux state, since they affect app behavior.

Some workarounds I have seen create hidden local state. For instance meatier's solution, its impure makeReducer function:

import {reducer as form} from 'redux-form'
import {compose} from 'redux'
import {combineReducers} from 'redux-immutablejs'
import auth from '../modules/auth/ducks/auth'
import landing from '../modules/landing/redux/landing'
import {routing} from './routing'

const currentReducers = {
  auth,
  routing,
  form,
  landing
}

export default (newReducers, reducerEnhancers) => {
  Object.assign(currentReducers, newReducers)
  const reducer = combineReducers({...currentReducers})
  if (reducerEnhancers) {
    return Array.isArray(reducerEnhancers) ? compose(...reducerEnhancers)(reducer) : reducerEnhancers(reducer)
  }
  return reducer
}

With this solution, you can't tell from the (serialized) state which reducers are active.

To me a more sophisticated solution would at least put some kind of keys in the redux state that identify which reducers should be loaded, even if the map of loaded reducers lives in local state.

@taion
Copy link
Contributor Author

taion commented Jul 13, 2016

cc @Scarysize @acdlite

How do y'all deal with this in redux-router?

IMO a router as currently implemented really should just be a reducer, but to give users maximum power from that, it'd really be best to be able to put the state in Redux.

@Scarysize
Copy link

We haven't really addressed the issue of unserializable state. Users have not complained, so either it wasn't relevant for them or they did find suitable workarounds.

I'm not really up-to-date with the current state of react routing solutions though. I guess @mjrussell may have a opinion on your issue as well.

In general I would prefer a reducer-based paradigm. As you already mentioned, it is much cleaner and easier to hook into. Also it is a well understood concept due to the popularity of redux.

@jokeyrhyme
Copy link

There have been a few recommendations to actually understand what a router does, and think about skipping react-router until its additional functionality is actually required:

Understanding something seems like a good idea, regardless. But would a reducer-style and/or serializable react-router help with keeping it simple or reduce (no pun intended) its magicalness / mysteriousness?

@jmurzy jmurzy mentioned this issue Sep 8, 2016
@timdorr
Copy link
Member

timdorr commented Sep 13, 2016

@taion's been working on an awesome history replacement that uses Redux under the hood: https://github.com/4Catalyzer/farce/tree/initial-impl Seriously, go check it out.

I think that might be a good avenue to head down, since the Router's own state is hard to serialize into a Redux state. I'm going to close this out in favor of that route (no pun intended).

@timdorr timdorr closed this as completed Sep 13, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants