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

meteorReduxReducers will be nested after persist/REHYDRATE? #27

Open
Diwei-Chen opened this issue Mar 26, 2017 · 6 comments
Open

meteorReduxReducers will be nested after persist/REHYDRATE? #27

Diwei-Chen opened this issue Mar 26, 2017 · 6 comments

Comments

@Diwei-Chen
Copy link
Contributor

Diwei-Chen commented Mar 26, 2017

111

Hi @JulianKingman ,

why would the meteorReduxReducers be nested stored after persist/REHYDRATE? If so, the size of meteorReduxReducers will theoretically be close to infinity if we keep refreshing the app?

Regards,

@JulianKingman
Copy link
Contributor

JulianKingman commented Mar 27, 2017

Ah, right. This is probably because combineReducers creates new nested objects with the reducer name, which is also probably why you're getting error #28. So we need to deal with this a little more intelligently, because combining reducers the way it's done now will break a bunch of things (see conbineReducers docs: http://redux.js.org/docs/api/combineReducers.html). Basically, if combineReducers is used, we need to use MeteorStore.meteorReduxReducers.dispatch(...) instead of MeteorStore.dispatch(...), make sense? Interested in creating a new pull request?

You might be able to get away with something as simple as:

// line 86
const newReducers = (customReducers !== undefined) ? combineReducers({ ...customReducers, meteorReduxReducers }) : meteorReduxReducers;
const MeteorStore = createStore(newReducers, preloadedState, enhancer);
if (customReducers) MeteorStore.dispatch = MeteorStore.meteorReduxReducers.dispatch;

@Diwei-Chen
Copy link
Contributor Author

Diwei-Chen commented Mar 28, 2017

@JulianKingman Sorry, I don't quite understand what's the difference between dispatched by MeteorStore and Meteor.meteorReduxReducers? Could you explain it with a bit more details, please? Also, we can't access to meteorReduxReducers as it is not directly under MeteorStore and it doesn't have a dispatch function as it is not a store object?

We tried very hard today and here are some screen shots:

screen shot 2017-03-28 at 4 48 16 pm

screen shot 2017-03-28 at 4 54 35 pm

We appreciate for your quick response :)

@JulianKingman
Copy link
Contributor

I'm sorry, I led you astray there, it's not that the store gets nested, just the state. So now whenever you call MeteorStore.getState(), you get {meteorReduxReducers: {...}, customReducers: {...}}. So on line 95, in the rehydration step, calling MeteorStore.getState() is getting the whole state object, it needs to be MeteorStore.getState().meteorReduxReducers instead.

So if you replace instances of MeteorStore.getState() with this, it should work:

const getState = typeof customReducers !== 'undefined' ? MeteorStore.getState().meteorReduxReducers : MeteorStore.getState();
_.each(getState, (collection, key) => {
// etc...

@Diwei-Chen
Copy link
Contributor Author

No worries. After added these two lines of codes, good news is, we can be able to restore all the customReducers' states properly, but all the previous states would be nested into meteorReduxReducers again. Then after several times of refreshing the app, the size of meteorReduxReducers will increase largely.

Is this problem caused by the 'rehydrated' step, which is from line 95 - 110? As the payload of current persist/REHYDRATE action doesn't overwrite prev state correctly?

@JulianKingman
Copy link
Contributor

How is this working in v2, I haven't tested?

@Diwei-Chen
Copy link
Contributor Author

Thanks @JulianKingman

screen shot 2017-05-18 at 11 33 02 am

This problem still exists

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants