search

ioof-holdings / redux-dynamic-reducer

Attach reducers to an existing Redux store
BSD 3-Clause "New" or "Revised" License
68 stars 9 forks source link

The nesting issue #1

Closed mpeyper closed 6 years ago

mpeyper commented 7 years ago

In the README there is a limitation listed that states:

Currently, reducers are only ever attached at the root of the store. Nesting is a complex problem we are working on. Components can be nested as deep as required, but the store state tree will not match the nesting structure. Consequently, the subspace keys must be unique across the board.

This issue if for anyone that wants to tackle that.

jcheroske commented 7 years ago

I'm starting to play with this lib now. Can you say more about the nesting problem and why it is complex? I'm wondering why you can't just use lodash set to create an arbitrary object graph? That's what redux-injector does.

mpeyper commented 7 years ago

I'm wondering why you can't just use lodash set to create an arbitrary object graph

Yep, that's how I solved it too.

The complexity comes from how you then combine the reducer tree. Imagine the following set of reducer attachments:

store.attachReducer('foo', fooReducer)
store.attachReducer('foo.bar', barReducer)

The problem here is that foo is not an object, it is a function, so combineReducers doesn't know how to deal with this. I've had a quick look at the redux-injector code and I don't think it will handle this case either. Unfortunately for us, this is the default scenario when mounting a withReducer component inside another one so we have no choice but to cater for it. Essentially it boils down to us having a more comprehensive combineReducersRecurse function.

The next challenge we have is caused by how react-redux-subspace works. As actions pass through subspaces, they get the namespace prepended to the action type. But the reducer that gets injected doesn't know about the other subspace it's supposed to be in, just the namespace it gets at it's level. so the action no longer finds it's with to the correct reducer. This was fairy easy to overcome with the v2 improvements made to redux-subspace though.

Shadowantz commented 6 years ago

I don't think that I need to open another issue so I am adding this comment here. The provided createStore will return a copy of the provided state every time attachReducers is used. The problem is that when I want to use immutable objects (using seamless-immutable for example) the 'immutable' properties are removed from the state. If I use store.attachReducer('foo.bar', barReducer), the 'immutable' props are removed from foo and bar. Is there a way to pass around this?

mpeyper commented 6 years ago

Hi @Shadowantz,

I think this warrants a seperate issue if there are changes required to support this, but we can discuss this here first to work out what's going on (although I'm happy for you to open an issue if you prefer).

The provided createStore will return a copy of the provided state every time attachReducers is used.

All attachReducers does is build the new reducer with new reducer included and call replaceReducer on the store, so any state changes (or copies) are the result of redux's behaviour when the reducer is replaced.

My feeling is that the issue will be being caused by how we construct the reducer and merge the state slices together when handling actions (for the same reason you need to use a special combineReducer function when using ImmutableJS) , but without seeing and example of how your store is setup and what your reducers look like, it's hard to tell.

Perhaps you can setup a simple example in codesandbox (or your sandbox of choice) and I can try to work it out?

Shadowantz commented 6 years ago

Ok, thanks for your answer! I will get back with an example when I will have time. But you got the idea :).