reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.88k stars 15.27k forks source link

Redux and immutable stores #548

Closed bunkat closed 9 years ago

bunkat commented 9 years ago

I'm starting to wrap my head around Redux and was previously using a basic flux architecture based around immutable stores. It seems like getting Redux to work nicely with an immutable store is a bit of work with lots of gotchas (from connect, to reducers, to selectors, to the dev tools, etc).

Since changes always flow through reducers and reduces shouldn't mutate state, are most people just going without an immutable store when they switch over to Redux? I'm concerned that this will make shouldComponentUpdate harder to write since you can't immediately tell what state has changed using strict equality which will lead to lots of unnecessary renders.

Was there a reason that Redux wasn't built with an immutable store?

acdlite commented 9 years ago

Redux makes no assumptions about the type of state you return from the reducer. You can use plain objects:

function reducer(state = { counter: 0 }, action) {
  switch (action.type) {
    case: 'INCREMENT':
      return { counter: state.counter + 1 };
    case: 'DECREMENT':
      return { counter: state.counter - 1 };
    default:
      return state;
  }
}

immutable data structures:

function reducer(state = Immutable.Map({ counter: 0 }), action) {
  switch (action.type) {
    case: 'INCREMENT':
      return state.update('counter', n => n + 1);
    case: 'DECREMENT':
      return state.update('counter', n => n - 1);
    default:
      return state;
  }
}

or any other value type, like a number:

function reducer(state = 0, action) {
  switch (action.type) {
    case: 'INCREMENT':
      return state + 1;
    case: 'DECREMENT':
      return state - 1;
    default:
      return state;
  }
}

Since changes always flow through reducers and reduces shouldn't mutate state...

In fact, that's exactly why immutable data and Redux go so well together. If you use persistent data structures, you can avoid excessive copying.

You may have misunderstood because of the way selectors work in react-redux. Because the result of a selector is passed to React's setState(), it must be a plain object. But this does not prevent the store's state from being an immutable data structure, so long as the selector maps the state to a plain object:

function select(state) {
  return {
    counter: state.get('counter')
  };
}

Hope that makes sense!

acdlite commented 9 years ago

This is a good topic for the docs

bunkat commented 9 years ago

Thanks for the explanation. Seems like there are a few other gotchas like Issue #153. Would be great to have a place in the docs for the proper way to take advantage of an immutable store.

There are also projects like redux-immutable which seems like they shouldn't even need to exist at all based on your comments. I'm also not sure how well things like redux-react-router or the redux-devtools or other middleware will work when the store is immutable. Then there is redux-persist which then needed redux-persist-immutable. And redux-reselect needs a custom comparator (maybe...). Basically every repo has at least one if not more issues raised with "how do I get this working with Immutablejs?".

Seems like everyone is touting the benefits of immutable stores, but in this case the path of least resistance is to just use plain objects instead.

acdlite commented 9 years ago

153 shouldn't be an issue. If you want the entire state tree to be an immutable object (rather than just the values returned by sub-reducers), it does require a bit more work, but nothing prohibitively difficult. You need to either use a specialized form of combineReducers() that returns an immutable value:

function combineImmutableReducers(reducers) {
  // shallow converts plain-object to map
  return Immutable.Map(combineReducers(reducers));
}

or eschew combineReducers() entirely and construct the top-level state object yourself.

As for the specific projects you mentioned, redux-devtools will work just fine regardless of the type of state. redux-react-router currently expects the router state to be accessible at state.router, but won't once this proposal is implemented. Not sure about the others.

bunkat commented 9 years ago

Ah, so that's what redux-immutable implemented. That makes sense now as does the proposal for redux-react-router. Be great if everyone that interacts with the store can be pushed in that direction. Appreciate all the help.

bunkat commented 9 years ago

Didn't mean to close if you are using this issue for docs.

danmaz74 commented 9 years ago

:+1: for adding a section in the docs for this - or maybe create a specific repository with all the instructions about how to use Redux with Immutable.js, and link it to the docs.

We also decided to use immutable.js, and for now what we did was to create a (very simple) custom combineReducers, didn't think about using the default one and then just transform it to Immutable.Map() Great suggestion @acdlite :)

ms88privat commented 9 years ago

Am I right to say that if you always return a new state in the reducer and therefore you don't mutate the state, it makes no difference to an immutable data structure performance wise? Or are there some other benefits beside less error prone?

danmaz74 commented 9 years ago

@ms88privat the performance advantage of using an immutable state is in the rendering of React components: with immutable state, you can compare references to learn if the state has changed inside shouldComponentUpdate, without any complicated (and potentially slow) deep comparisons.

leoasis commented 9 years ago

@ms88privat as @acdlite said in this comment, the improvement of using immutable data structures over plain js objects and arrays is the ability to reuse the unchanged things as much as possible, and avoiding excessive copying. By assuming the data is immutable, the library can do structural reusing of the collection, and share the unchanged parts between different objects, and still making it look to the outside as completely different structures.

@danmaz74 the performance improvement you mentioned also applies to using plain objects or arrays without mutating (ie creating a new state with the changes). The difference between something like immutable-js and POJOs is what I described in the previous paragraph.

ms88privat commented 9 years ago

@leoasis Thats what i wanted to hear. So react performance wise it should be the same, but it has less copying / memory-footprint or something like that.

What is the default status of shouldComponentUpdate with Redux? I read something about pureRenderMixing ... which i think should apply to redux, like we said. But how to implement in ES6?

leoasis commented 9 years ago

@ms88privat react-redux's connect method creates a Connect higher order component that implements shouldComponentUpdate: https://github.com/gaearon/react-redux/blob/master/src/components/createConnect.js#L82-L84

As per your last question regarding ES6 and pureRenderMixin, that mixin is just implementing shouldComponentUpdate as it is done in the Connect component above. So that's really what you need to do if you want to have the pure render behavior in your ES6 component classes.

ms88privat commented 9 years ago

@leoasis thx, makes sense - so it's good to go.

danmaz74 commented 9 years ago

@leoasis: @ms88privat wrote: "If you always return a new state in the reducer and therefore you don't mutate the state, it makes no difference to an immutable data structure performance wise". That's not correct, because if you always return a new state in the reducer (ie, also when there is no change) you can't just compare the reference - the comparison will always return the "new" state is different from the "old" state. That is in addition to the fact that using Immutable to create a new changed state is faster than creating a new deep copy.

leoasis commented 9 years ago

@danmaz74 Well, if you do something like this:

return {...state, something: 'changed'}

then the root state and the something property will be the only different objects, while the other properties in state will be the exact same reference and will be able to take advantage of the shouldComponentUpdate optimization.

It is true though that the Immutable.js library checks if the value you are setting is already set, and does nothing if that's the case, but that's fairly simple to do with POJOs as well:

return state.something === 'changed' ? state : {...state, something: 'changed'}
ms88privat commented 9 years ago

@danmaz74 Ok, i have to correct my spelling. If there is no change at all (= no action), the reducer will return the last state and there is no need for a new state.

danmaz74 commented 9 years ago

@ms88privat ok no problem, I just interpreted that literally.

But in using Immutable (or something similar), there's still the advantage of easily getting what @leoasis pointed out in his last comment - without a library you can still do it using using ES7 spreads, but it looks much more complicated/less maintainable that way (at least to me).

gaearon commented 9 years ago

We need “Usage with Immutable” in docs Recipes folder.

jonathan commented 9 years ago

@gaearon I agree. I started playing with redux over the weekend trying to port a current app over from a flux implementation. I've been slowly figuring out how to use immutable but hit snags in the reducers and the tests for them. I've been able to use chai-immutable for my tests (https://github.com/astorije/chai-immutable) but it took some bumbling around by me to figure it out.

That being said, it hasn't been a terrible burden to figure out. It would just be nice to have a smoother onramp.

danmaz74 commented 9 years ago

@acdlite after a deeper analysis, unfortunately you can't simply do Immutable.Map(combineReducers(reducers)); because combineReducers() returns a function, not an object. This is less terse, but works:

let combineImmutableReducers = reducers => {
  var combined_reducers = combineReducers(reducers);

  return (state,action) => Immutable.Map(combined_reducers(
      Immutable.Map.isMap(state) ? state.toObject() : state,action
  ));
}
acdlite commented 9 years ago

@danmaz74 Yes you're right, my mistake. It was meant to be a naive example; e.g. even with your fix, it returns a new Immutable map every time, regardless of whether any of the keys have changed. A more complete implementation would update the previous state map rather than creating a new map. I think using map.merge() would suffice.

danmaz74 commented 9 years ago

@acdlite You're right about the new map every time, but I think Map#merge always returns a new map anyway, too (at least, that's what the docs say). Edit: I just did a test and the map is the same if the merge doesn't change anything.

Before testing the code above, we used this one:

let app_reducers = (state=Immutable.Map({}), action={}) => {
  for (let reducer_name of Object.keys(reducers)) {
    state = state.set(reducer_name, reducers[reducer_name](state.get(reducer_name), action));
  }
  return state;
};

This wouldn't change the root state if nothing changes below, but it wouldn't do the checks that combineReducers does. Honestly it's not clear to me which approach would be better; I guess that changing the root map would usually do very little damage, but of course it wouldn't be 100% clean. Any thoughts?

dfcarpenter commented 9 years ago

I also would love more documentation/examples on using immutable.js with Redux. Also, has anyone used normalizr with immutable.js? I was thinking of using it in some api middleware and was wondering if it was viable.

gaearon commented 9 years ago

@dfcarpenter Normalizr is mostly useful for normalizing API responses before they become actions, so there's no difference: you can use it regardless of what you use for state.

chollier commented 9 years ago

@dfcarpenter We do use normalizr with redux, I have a createEntityReducer factory :

import Immutable, { Map } from "immutable";

export default function createReducer(reducer, entitiesName) {
  return (state = new Map(), action) => {
    const response = action.response;
    const data = response && response.data;
    const entities = data && data.entities;
    const collection = entities && entities[entitiesName];
    if (collection) {
      return reducer(state.merge(Immutable.fromJS(collection)), action);
    } else {
      return reducer(state, action);
    }
  };
}

I realize now that I could probably improve it by making it a middleware to which I would pass all my schemas..

To not be too much off topic, I'm also really interested in having redux working with Immutable from the grounds up. I like redux-immutable but I'm not a fan of having to use https://github.com/gajus/canonical-reducer-composition by default..

kmalakoff commented 9 years ago

@chollier I was wondering if you can confirm if I understand correctly about using both normalizr and Immutable.js with redux.

I am just starting to get deeper into React/Flux, but it seems like the benefit of Immutable.js with React is to reduce graph traversal through something like the PureRenderMixin. If your store stores each model independently/unlinked using normalizr with references through ids to other models, it seems like there is little opportunity to know if the subgraph needs rendering (for example, maybe only at the leaves) since Immutable.js will not actually store the graph, but the individual links so the simple immutable instance checks will not cover the changes lower in the graph.

Another way to ask this is: are there any common ways to optimize React rendering using normalizr or do you have to build the full virtual DOM whenever changes occur in a hierarchical model graph?

cesarandreu commented 9 years ago

One small potential gotcha that you might encounter when using normalizr, immutable.js and numeric IDs...

If you have a collection: [{ id: 1, name: 'A' }, { id: 2, name: 'B' }]

normalizr will generate something like: { 1: { id: 1, name: 'A' }, 2: { id: 2, name: 'B' } }.

If you pass normalizr's output to Immutable.fromJS, you'll only be able to access the value with the string ID. Maps differentiate between numbers and string: collection.get(1) !== collection.get('1').

This can lead to unexpected bugs, for example you might have seemingly harmless code like:

const user = userCollection.get('1')
const account = accountCollection.get(user.get('account'))

But that won't work, and account will be undefined.

chollier commented 9 years ago

@kmalakoff I think one answer to that is to use cursors.

Also because of the way immutable.js store things, even if part of your tree change, you can still get equality comparison on other parts of the tree. Example :

> a = Immutable.fromJS({ a: 1, b: 2, c: { d: 4, e: 5, f: [6, 7]}})
Map { a: 1, b: 2, c: Map { d: 4, e: 5, f: List [ 6, 7 ] } }
> var c = a.get('c')
undefined
> c
Map { d: 4, e: 5, f: List [ 6, 7 ] }
> c == a.getIn('c')
true
> a = a.setIn('a', 12)
Map { a: 12, b: 2, c: Map { d: 4, e: 5, f: List [ 6, 7 ] } }
> c == a.getIn('c')
true
Haotian-InfoTrack commented 9 years ago

interesting topic. Would like to see more examples.

geminiyellow commented 9 years ago

Yeah,need more examples for immutable and normalizr

kmalakoff commented 9 years ago

@chollier makes sense. From what I understand with normalizr and your example, if the List in 'f' referred to related models in the store, you would only be able to check if the reference to the ids changed, but not is the underlying models changed (since they are not embedded in 'f') so you wouldn't know if the tree needs rendering and would need to render it regardless of whether the ids changed, right?

gaearon commented 9 years ago

Closing, as there doesn't appear anything actionable for us here. If somebody wants to write “Usage with Immutable” please do! Unfortunately I don't have experience in using them together so I can't help.

asaf commented 9 years ago

More references:

Plain Redux with ImmutableJs: https://github.com/indexiatech/redux-immutablejs

A project that combines Redux with Seamless-Immutable: https://github.com/Versent/redux-crud

cjke commented 8 years ago

Although this was closed back in September, I still haven't found any clear examples of using Redux with ImmutableJS, especially when it comes to shouldComponentUpdate and with normalised data.

Demo Please find a fun little JSbin here: http://jsbin.com/tikofel/5/edit?js,console,output

I think I was initially hitting the same hurdle that @kmalakoff was touching on. That with the normalised data, how does the container record "know" to update when one of it's children update - as it's just a sequence of numbers/strings/ids.

In the example above, I have created the data in a normalised way, but could also be achieved using Dan's nifty normaliser library.

Notes

I feel like this is the right approach, but am still learning redux/react/immutable, so would love to get anyones feedback. It's not a recipe as @gaearon mentioned, but hopefully this is on the right track.