gajus / redux-immutable

redux-immutable is used to create an equivalent function of Redux combineReducers that works with Immutable.js state.
Other
1.88k stars 82 forks source link

How to add non immutable store when combining reducers #69

Open kopax opened 6 years ago

kopax commented 6 years ago

Hi, thanks for redux-immutable.

I am trying to mix immutable reducer and classic reducer.

I have tried to tweak the combineReducer.js like this :

    if (inputState.withMutations) {
      return inputState.withMutations(function (temporaryState) {
        reducerKeys.forEach(function (reducerName) {
          var reducer = reducers[reducerName];
          var currentDomainState = temporaryState.get(reducerName);
          var nextDomainState = reducer(currentDomainState, action);

          (0, _utilities.validateNextState)(nextDomainState, reducerName, action);

          temporaryState.set(reducerName, nextDomainState);
        });
      });
    } else {
      reducerKeys.forEach(function (reducerName) {
        var reducer = reducers[reducerName];
        var currentDomainState = inputState[reducerName];
        var nextDomainState = reducer(currentDomainState, action);

        (0, _utilities.validateNextState)(nextDomainState, reducerName, action);

        inputState[reducerName] = nextDomainState;
      });
      return inputState;
    }

And mix my reducer like this

import { combineReducers } from 'redux';
import { combineReducers as combineReducersImmutable } from 'redux-immutable';

export default function createReducer(injectedReducers) {
  const immutableReducer = combineReducersImmutable({
    route: routeReducer,
    locale: localeReducer(),
    form: formReducer,
    ...injectedReducers,
  });
  const nonmutableReducer = combineReducers({
    admin: adminReducer,
  });
  return combineReducersImmutable({
    admin: adminReducer,
    route: routeReducer,
    locale: localeReducer(),
    form: formReducer,
    ...injectedReducers,
  }, () => {
    return {
      ...immutableReducer(undefined, { type: '' }),
      ...nonmutableReducer(undefined, { type: '' })
    };
  })
}

But that doesn't work very well. Any Idea how I could achieve this ?

cramatt commented 6 years ago

I'm having this issue too, trying to use https://github.com/KELiON/redux-async-initial-state/issues/15. Any suggestions @gajus ?

rahamin1 commented 5 years ago

@kopax @cramatt Have you found a solution?

kopax commented 5 years ago

We stopped using redux-immutable. Instead, if a library provide support for redux-immutable, we can still plug a redux-immutable reducer into the root, non immutable store.

I will recommend to never use a root immutable store.

This should be written in best practice.

rahamin1 commented 5 years ago

@kopax Thanks for your reply.

Any idea how to do it? I tried but failed. See my question here: https://stackoverflow.com/questions/55852422/combining-immutable-reducer-regular-reducer

gajus commented 5 years ago

I will recommend to never use a root immutable store.

I have not used Redux for a long time. Can you explain what is the reason you do not recommend having root store immutable? This suggestion makes sense if there is a requirement for interoperability with third party libraries.

rahamin1 commented 5 years ago

I know that the question is directed to @kopax, for me either way is fine, as long as I can mix immutable with regular objects.

kopax commented 5 years ago

@rahamin1 you must use the combineReducer from redux and you will have a root non immutable.

@gajus ImmutableJS is a smaller community, it splitted the redux community which is bad and developer will have to support both (Like old times with IE). We developers want to keep things simple and this could have been solved long ago if the combineReducers from this package would have never existed.

If you get my point, I advise that you remove it and you should see the redux-immutable community growing.

gajus commented 5 years ago

@gajus ImmutableJS is a smaller community, it splitted the redux community which is bad and developer will have to support both (Like old times with IE). We developers want to keep things simple and this could have been solved long ago if the combineReducers from this package would have never existed.

I don't get your point. Are you suggesting not to use ImmutableJS at all?

kopax commented 5 years ago

What I mean is If you do use import { combineReducers } from "redux-immutable"; to create the root store then you are doing wrong.

I am suggesting to not suggest that to users.

gajus commented 5 years ago

I understand what you are suggesting, I don't understand your arguments.

I am not defending redux-immutable. I am not even using redux anymore. If there is a valid reason to deprecate redux-immutable, then I am happy to do it. But it cannot be a because of style preference or community size; it needs to be a well argumented technical reason, such as incompatibility with X, Y, Z.

kopax commented 5 years ago

@gajus, the main concern is how to join the two community.

I see it fairly simply. If you do have a root immutable store, then it will not happen.

You should suggest users to use combineReducer from redux and never use fromJS on the root store for that reason.

edit

I missed the interesting part of your message, why aren't you using both redux and redux-immutable anymore?

gajus commented 5 years ago

I missed the interesting part of your message, why aren't you using both redux and redux-immutable anymore?

There is really no need for Redux. By now I have oversaw development of/ developed hundreds of React apps, and the benefits provided by Redux (to an avg. size application) are incredibly marginal compared to the boilerplate that is required to implement it. I use react-apollo + useState.

msageryd commented 2 years ago

@kopax Would you mind sharing how managed to stop using redux-immutable? I want to get out of it too, but I need to convert this live in production, i.e. I would like to read the store as Immutable and write it back as JS. I will still have a bunch of Immutable slices, those will be converted later. The root is the biggest problem.

AlexeiDarmin commented 2 years ago

@gajus I'm currently looking to migrate from immutable to immer, and as a consequence I'll be migrating a store towards one that has some slices in ImmutableJS, and others in vanilla JS. The code base is huge and it would be impossible to do a 100% overhaul safely.

Ideally, the combine-reducers function would be able to identify if the slice is immutable or JS and handle it accordingly.

Can you explain what is the reason you do not recommend having root store immutable? This suggestion makes sense if there is a requirement for interoperability with third party libraries.

I'm early in my investigation but, if the root store must be immutable then it introduces a few challenges:

I'll investigate the following comment and see what the process looks like to migrate from a pre-existing redux-immutable store to a partial immutable partial vanilla store.

We stopped using redux-immutable. Instead, if a library provide support for redux-immutable, we can still plug a redux-immutable reducer into the root, non immutable store.

I will recommend to never use a root immutable store.

This should be written in best practice.