Comments (12)
You could just define a new mapping function that uses .forEach()
and .set()
function keepReferenceMap(state, fun) {
let newState = state
state.forEach((val, key) => {
newState = newState.set(key, fun(val))
}
return newState
}
It is a little less elegant than just calling .map()
but not that bad:
import { keepReferenceMap as map } from 'keep-reference-map'
...
state = map(state, val => {
return 8;
})
from redux-immutablejs.
@davidmason I don't think this solves the problem as calling .set() is also going to return a new object so newState !== state.
I am thinking that the only way that state === newState after running the reducer is if the reducer returns the state object it was given. So we should check for that and only mutate our state if the reducer has.
finalReducers.forEach((reducer, key) => {
var oldState = state.get(key);
var newState = reducer(oldState, action);
if (typeof newState === 'undefined') {
throw new Error(getErrorMessage(key, action));
}
// Check to see if this reducer mutated the state
if (newState !== oldState) {
state.set(key, newState);
}
});
// If none of the reducers mutated the state then we are returning the same object so any === checks will work
return state;
from redux-immutablejs.
@kainoa21 I thought immutablejs kept reference equality with .set
when oldval === newval
, if not then I agree with that check - actually I used exactly that in a proposed fix for the React immutability helper to keep reference equality: facebook/react#4968
from redux-immutablejs.
@davidmason Yep, .set()
already does this check, so there is no need to do it manually. (See here for an example illustrating just that)
Thinking about this more, it seems like there are a few important considerations here:
- (Most important) Are people actually "selecting" the slice of the state that is returned by
combineReducers
?- I actually jumped the gun opening this issue, because in my own application I realized I never actually access this "top-level"
Map
directly - I just create more specific "selector" functions which access the children of thecombineReducers
stateMap
anyway - The
reselect
memoize functionality doesn't care if the parent has changed or not – only the specific item you "select" – so there is no real downside to using.map()
to implementcombineReducers
if you aren't "selecting" thisMap
directly
- I actually jumped the gun opening this issue, because in my own application I realized I never actually access this "top-level"
- How many reducers are people typically "combining" with
combineReducers
?- Since each call to
.set()
could potentially allocate a new object, there is a performance benefit to batching these into a single allocate (which is exactly what.map()
and.withMutations()
do) - If there are a lot of reducers being "combined", then that means a lot of calls to
.set()
, and thus potentially a lot of object allocations (which is slow), but if it's only a few then the performance difference is likely negligible - Also, if the answer to question (1) is "no" then the clear winner is to use
.map()
or.withMutations()
regardless of how many reducers people are typically "combining"
- Since each call to
from redux-immutablejs.
- I imagine that the most common usage will be as you described with selectors accessing the various domains which are the input reducers to combineReducers. But there are certainly use cases for selectors that will look at the root state object.
- I think we could rephrase the question to be: How many reducers are actually mutating state for a given action?
- Since each call to
.set()
will only allocate a new object when the state has been mutated, then this is really the performance impact to consider - I think the answer here is very likely to be that a single reducer is mutating the state for a given action, or at the very least a small number (not likely the entire list of reducers being combined).
- Even then, one possible solution to mitigating performance impact would be to simply wrap all of the necessary
.set()
calls inside a.withMutations()
- Since each call to
- Another consideration: How likely is it that an action will NOT mutate the state whatsoever?
- This is the only scenario in which using
.set()
over a simple.map()
could potentially provide any benefit. And even then, that would be further limited to scenarios where selectors are accessing and doing a reference equality check on the root state object.
- This is the only scenario in which using
from redux-immutablejs.
I would expect to see multiple levels of combineReducers()
in non-trivial apps, at least some of the time, but I don't see that being relevant to .map()
I guess the bottom line is that there may be different performance characteristics between the current .map()
and a different strategy using .set()
and .withMutations()
. I think a sensible next step would be to compare the performance of .map()
and the other strategy across a range of scenarios, and continue from there.
Something like this would be the thing to compare:
function keepReferenceMap(state, fun) {
return state.withMutations(function (state) {
var newState = state
state.forEach(function (val, key) {
newState = newState.set(key, fun(val))
}
return newState
}
}
Are there already performance tests for immutable? If so, it would just be a matter of replacing .map()
and running them.
from redux-immutablejs.
.withMutations always returns a new object, so it wouldn't really be of
much help here FYI
On Sep 27, 2015 4:36 PM, "David Mason" [email protected] wrote:
I would expect to see multiple levels of combineReducers() in non-trivial
apps, at least some of the time, but I don't see that being relevant to
.map()I guess the bottom line is that there may be different performance
characteristics between the current .map() and a different strategy using
.set() and .withMutations(). I think a sensible next step would be to
compare the performance of .map() and the other strategy across a range
of scenarios, and continue from there.Something like this would be the thing to compare:
function keepReferenceMap(state, fun) {
return state.withMutations(function (state) {
var newState = state
state.forEach(function (val, key) {
newState = newState.set(key, fun(val))
}
return newState
}
}Are there already performance tests for immutable? If so, it would just
be a matter of replacing .map() and running them.—
Reply to this email directly or view it on GitHub
#11 (comment)
.
from redux-immutablejs.
.withMutations always returns a new object
It sounds like there is room for a review of immutable's API to assess it specifically for use-cases that want to use reference equality - initially to make a reference doc for anyone wanting to use reference-equality, then to figure out if there is a set of changes that would help support the use-case while having a neutral or positive effect on the other uses of immutable.
One use-case that would benefit from maintaining reference-equality is React with the pure render mixin. Another is using reselect to memoize state-selecting functions.
from redux-immutablejs.
I ran into this issue as I was creating a history reducer. I only wanted to push a new history when the state changed, but because a new object was returned every time, my history was getting cluttered with entries that didn't actually represent changes.
Just created a pr #15 which addresses this issue.
from redux-immutablejs.
Folks, sorry for responding within a delay, holidays in Israel :-)
I wasn't involved in this discussion but it seems like @dgreisen-cfpb (thanks!) PR solves this issue, I merged it and released a new version.
Thanks !
from redux-immutablejs.
@idolize Are you feeling comfortable with this solution?
from redux-immutablejs.
@asaf Yeah 😄 Thanks everyone for the discussion, and thanks @dgreisen-cfpb for the simple (but effective) solution!
from redux-immutablejs.
Related Issues (20)
- Having trouble providing initial state to reducers HOT 2
- Not installable from Git HOT 3
- redux 4.0 HOT 1
- Issues with initialState when using combineReducers HOT 1
- Can not work with redux-router HOT 9
- reducer must return immutable, how to handle js immutable types? HOT 2
- Is this still needed? HOT 2
- npm package cannot be used with Babel 6 HOT 1
- Steps to make it work with 'react-redux-universal-hot-example' HOT 1
- Consider deprecating in favour of redux-immutable HOT 4
- If I use the redux-immutablejs' createReducer(), will I still get back the initial state if the action is undefined as I would with a switch statement? HOT 1
- Discrepancy between github repo and published npm version HOT 3
- understanding effective use of (use-cases for) redux-immutablejs
- How to work with react-router-redux?
- combineReducers to return Immutable.Record instead of Immutable.Map
- TypeScript support?
- Link in README.md invalid
- OBSERVATION: fromJS in combineReducers not successfully converting to Map in node (but works in browser)
- Upgrading to immutable 4 HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
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.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from redux-immutablejs.