RFC: syncHistoryWithStore()

[gaearon]

As discussed in reactjs/react-router-redux#257, some features of react-router-redux such as dedicated action creators do not seem necessary with the API changes in [email protected]. In addition, it seems to me that middleware is suboptiomal API for the DevTools replay functionality.

This pull request is an attempt at starting a discussion around what react-router-redux could look like if instead of using middleware, we wrapped the history object itself. To me, this seems a more natural approach to this problem, and seems to fix some issues with the current DevTools support such as reactjs/react-router-redux#252.

In this PR we:

• Remove dependency on react-router-redux and use react-router as is
• Demonstrate how we can preserve record/replay/time travel features while using vanilla history API
• Implement a function that is similar in spirit to the original redux-simple-router in terms of its simple API

If there is interest on react-router-redux side and there are no apparent problems with this approach, we may want to use that as its evolution. Otherwise we may publish something like this independently. I have not considered server rendering here, so please let me know if I am way off, and this approach is broken in some way.

Finally, let me acknowledge that I took 80% of the logic from react-router-redux. My only contribution here is slightly tweaking the API to emphasize vanilla react-router usage and fixing some edge cases in DevTools support.

I will link to this PR on react-router-redux as well. Big thanks to @jlongster, @taion, @timdorr, and @naw for their work and help with this.

Related issues:

• Make redux drive history? reactjs/react-router-redux#138
• Alternative Proposed Solution For React-Router + Redux Integration reactjs/react-router-redux#248
• Restore router state reactjs/react-router-redux#252
• [WIP] Tweak the API to handle initial state correctly reactjs/react-router-redux#255
• Clarifying the goals of this project reactjs/react-router-redux#257

[timdorr]

https://github.com/rackt/redux/blob/routing-history/examples/real-world/syncHistoryWithStore.js

(Just posting so I have quick access to take a look at the important file 😄)

[gaearon]

(Just posting so I have quick access to take a look at the important file 😄)

Also notice how the usage changed. There is no middleware or store enhancer, it works as a single function call:

const store = configureStore()
const history = syncHistoryWithStore(browserHistory, store, {
  adjustUrlOnReplay: true
})

You still do need to mount the reducer but you can use the regular history rather than the special action creators.

[timdorr]

Since there are no action creators exposed and the state key is named to discourage access, it would seem the primary purpose is to sync up the URL bar after a replay. In that case, should this default to true?

[gaearon]

Defaulting to true means that if you hydrate state from e.g. localStorage URL will adjust to that initial state. It can be desirable in development and in production "replay" scenarios but it might be surprising to people who just store some Redux state in localStorage. This is why I'd rather see it opt-in.

[timdorr]

You can detect that, because the initial location on load is a "POP" action.

[gaearon]

How would I tell between the first POP action and a persisted session that happens to end on a POP?

[timdorr]

I'm confused. Why would it be surprising that restoring your serialized state, which syncs to history, would restore your URL bar? Wouldn't that be an expected behavior?

[timdorr]

You still do need to mount the reducer but you can use the regular history rather than the special action creators.

Why not make this a store enhancer so the user doesn't have to do that?

[timdorr]

Ah wait, hit enter too soon. We're not returning the store, so we can't enhance it. Hmm...

[gaearon]

Why not make this a store enhancer so the user doesn't have to do that?

How would we inject a reducer into an existing reducer combination? Also some people want to use Immutable so it's not very good to assume a specific way of putting it there. And wrapping all app's state just because we want to "own" routing key is not very good either. I think this is the lesser evil here.

[gaearon]

TODO (optimization): check if location has changed and don't call listener if it hasn't.
Should be added after tests are in place.

[timdorr]

How would we inject a reducer into an existing reducer combination?

The only way I can think of is the same way applyMiddleware does it, by wrapping dispatch. Not that we should actually do that, of course.

I'm just looking out for the person that forgets one part of the install process and misses a key reducer. Then when they click on stuff nothing happens. Doesn't really fail gracefully.

[taion]

Could you make this a store enhancer that just overwrites history.listen in-place? Mutating history isn't the cleanest thing in the world, but it might work.

[gaearon]

I'm just looking out for the person that forgets one part of the install process and misses a key reducer. Then when they click on stuff nothing happens. Doesn't really fail gracefully.

Fixed in 0db894c.

[gaearon]

Could you make this a store enhancer that just overwrites history.listen in-place? Mutating history isn't the cleanest thing in the world, but it might work.

What would be the point of making it a store enhancer? What problem would it solve?

[timdorr]

Less wiring up by the user is the main purpose. We can also better hide the location state.

[taion]

@gaearon My #1362 (comment) was in response to @timdorr's #1362 (comment).

[taion]

Better than that though would be a

const { history, storeEnhancer } = wrapHistory(history);

or something.

[gaearon]

cc @ezekielchentnik as this is conceptually similar to his redux-history project but slightly more ambitious (DevTools work + store is treated as primary source of truth rather than a copy). Maybe we can publish this as evolution of redux-history? There doesn’t seem to be anything specific to React Router here, and maybe it doesn’t make sense to call this react-router-redux. I know nobody is happy with the churn, so maybe we can see which approach wins in the long term, and avoid breaking react-router-redux users.

[timdorr]

On the naming part: React Router 2.0 hides the History project a little better. You're still talking to history objects, but they're provided via the Router's APIs. And there's no peerDep on history. So, it's visibility is going away and I think most people, from a "brand" perspective, are going to associate this with React Router vs History.

I would be interested in taking react-router-redux in this direction. We can split it into two exports: The action creator/middleware part, and the reducer/history sync part. Import whatever part you need. If we can get some code contributions from @ezekielchentnik or @taion or you, that would be fantastic. I'm just a steward of the project, so it's whatever the community makes of it.

[taion]

redux-simple-router has never actually dealt with React Router, though.

react-router-redux doesn't strictly make sense as a name, but it's probably the least confusing name for most users.

[timdorr]

No, but the brand association is there. It's purely a naming thing, now that history is fading more into the background.

[gaearon]

I’m happy to contribute to react-router-redux but I heard concerns from @jlongster about API churn in a (not very good) initial PR I sent a couple of days ago. I don’t want to impose my opinions on this project. However I truly believe its current architecture doesn’t fit Redux as well as this proposal, so this is how I started thinking about maybe starting another library. If react-router-redux maintainers are willing to change its API again, I’m happy to find some time to contribute this back within the next couple of weeks.

[benmonro]

why not just name the thing 'routing' in syncHistoryWithStore? I do this exact same thing in my code and my hunch is that other people do as well. 😄

[timdorr]

I don't consider the API churn as big of an issue as @jlongster might. shrug It would be different if we were just moving things around for the sake of moving them around, but this is a big philosophical shift for the library. And I think it's a good one that educates users about the APIs they should be using, rather than hopping onto a cargo cult library that it's going to turn into with Router 2.0. I would like to make it a history syncing library first and "oh yeah, hey, it also does this action creator stuff, but don't worry too much about that because Router has some great new APIs!" It's really Router and History 2.0 that is driving a lot of this. We should be keeping up with them, rather than just applying patches to translate old patterns to new APIs.

I'm going to get started on something like that tonight to get the ball rolling on discussion and improvements. This has already been a great move forward for the library and I really appreciate you taking the time to look into this with such detail, @gaearon!

[jlongster]

I'm definitely ok with another API change, I'd rather that than starting yet another project. My initial implementation was super, super simple and only aimed to solve the serialization problem. In the side project that I used it in, I only used react-router's APIs. As the project gained popularity, a lot of bugs were exposed mainly because of people trying to use APIs outside of react-router (the react-router-redux APIs to read and control location). I've been supportive of refactorings that supported those use-cases more, but I've never loved the slow movement towards "make redux drive everything".

This is much more in line with the roots of the project. I'm happy to see it. I think we'll get some pushback from users, but that shouldn't stop us from doing what we think is the best. Careful thought towards how we communicate this would be good.

I'm in the middle of a bunch of stuff at work, and my wife is about to have a baby any moment now, so I can't look too closely at this. But from what I see I like it, and I defer many of the details to you all. I'll try to take a closer look over the next few days if I can.

[gaearon]

@jlongster

Thank you for taking your time to respond! We’ll definitely think about communicating this correctly and treading lightly. Moving action creators and middleware to a separate export from history enhancer might be the best idea in terms of casing less churn overall while advancing the project.

[jlongster]

@gaearon "separate export"? I hope you don't mean separate project. There are already a lot of small moving pieces in the react/redux ecosystem. If you mean still provide them in the library as something that can be used, that sounds good, and we can deprecate over a release or two and remove them eventually.

[timdorr]

@jlongster I'm thinking something like import { syncHistoryToStore, routerReducer, routerMiddleware, routerActions } from 'react-router-redux'

[gaearon]

"separate export"? I hope you don't mean separate project.

I was thinking something like

import { push, replace, middleware } from 'react-router-redux/actions'

and

import { reducer, keepStateInStore } from 'react-router-redux/sync'

but maybe it doesn’t really make sense in ES6 land.

What I’m trying to communicate is the “action creators + middleware to interpret them” and the “history enhancer + reducer” are two separate parts of the project that can be used independently. You don’t have to use them both.

[jlongster]

I see, great. That sounds much better than cutting users off from the way they currently do things in one release.

[taion]

Do we really need the actions when we can do browserHistory.push? I can come up with examples where they might be necessary, but I'm not convinced we need them in most cases.

[gaearon]

Do we really need the actions when we can do browserHistory.push? I can come up with examples where they might be necessary, but I'm not convinced we need them in most cases.

I agree they are not necessary. However, as I gathered from reactjs/react-router-redux#257, people do rely on them for several things:

• performing history actions on non-singleton history object when data fetching without passing it from the context
• asserting that correct actions have been dispatched—easy to test return values of action creators
• logging them as they go through other middleware

So I wouldn't want to completely remove them—only to de-emphasize them and explain that they are not necessary. As we figure out pain points and address them in React Router eventually we may deprecate them, but we need to make sure we’re not leaving the users out in the cold for scenarios they care about and that aren’t easy to codemod.

[jlongster]

Hmmmm that's a good point. Is there nothing indication the internal routing state changes in your new approach? Does that mean given some state and some actions, we can no longer reliably replay those actions and generate the same state all the time?

[timdorr]

@jlongster There's a dispatched action and the type is exported as well, so you can look for it in your middleware.

[gaearon]

The way it works now, if you use syncHistoryWithStore(history, store) there is both state and actions, the user just doesn’t trigger them:

function syncHistoryWithStore(history, store) {
  // ...

  history.listen(location => {
    // ...
    store.dispatch({ type: NAVIGATE, location })
  })

  // ...
}

This is how this PR allows users to interact with history object and still get full record/replay.

---

In this case, providing { push, pop } would be just a convenience but they wouldn’t be actual actions. They would just be instructions to the (optional!) middleware that would interpret them and call history methods for the user. At this point, if the user is using syncHistoryWithStore(), they will trigger real actions and cause store updates.

[jlongster]

@gaearon Nice, thanks for explaining. I guess I just need to look at this closer which I'll do soon.

[gaearon]

This is why I’m stressing that these are two complementary but separate things.

---

(Not in this PR:)

react-redux-router/actions exports push, pop, etc, and middleware(history). All that middleware would do is to convert dispatch(push(stuff)) into history.push(stuff). This solves the problem of dependency injecting history and making action creators testable without any connection to record/replay functionality. In fact those “actions” would never even reach the reducers.

---

(In this PR:)

react-redux-router/sync is when you want Redux to store your routing state. It exports reducer and syncHistoryWithStore(history, store). It does not know anything about the any of the stuff in the first paragraph. syncHistoryWithStore will return you an object that behaves like history (has same methods) but is backed by Redux store. For example, enhancedHistory.listen() actually becomes store.subscribe().

Under the hood, it subscribes to the passed history once, and rather than call the subscribers on change, it dispatches an action. Essentially it “routes” history updates through Redux so that the store becomes a source of truth for anything interacting with such enhancedHistory really interacts with the store. React Router will use such history and when we toggle actions in DevTools, it will genuinely think that history is changing, and adjust the UI.

DevTools compatibility and record/replay naturally fall out of this, with some manual mode to correct the URL bar. Of course in this case you would need to mount the provided reducer into your tree.

---

These two things can work together but I want to highlight there is no shared code or dependencies between them. You can use one, other, or both.

[timdorr]

Opened up reactjs/react-router-redux#259 to build this out on react-router-redux

[ezekielchentnik]

@gaearon @timdorr I'm down to contribute and collaborate any way I can. From my perspective, I've always seen history functionality completely independent of react-router (although, likely to be used together). From experience, I've worked on projects that don't need full featured react-router, but could still take advantage of stuffing history/location into the store.

What I’m trying to communicate is the “action creators + middleware to interpret them” and the “history enhancer + reducer” are two separate parts of the project that can be used independently. You don’t have to use them both.

I agree completely with this sentiment. Allowing a user to piece things together in a simple way can go along way for adoption and account for multiple uses (what if react-router is no longer defacto? what if you only need a query param for a fetch? what if you're not using react?).

I've got some code laying around where I did split subscribing to store, and listening to history so they could be used independently without it being awkward, but had trouble deciding on the api naming ... I'll try to get a PR/gist to continue discussion. Cheers.

[gaearon]

This is now superseded by reactjs/react-router-redux#259 so let’s continue the discussion and contributions there.

Thank you to everyone, and looking forward to [email protected].