Redux - Devtools are broken in release 2.0.1

[winklerp]

The new ModuleEnhancer is composing the enhancers without redux devtools by using the plain compose method of redux and so the store is not recognized in the tools.

Fix in ModuleEnhancer.ts:

let composeEnhancers = compose;

if (process.env.NODE_ENV === "development") {
composeEnhancers = window["REDUX_DEVTOOLS_EXTENSION_COMPOSE"] || compose;
}

[navneet-g]

Thanks @winklerp for reporting the issue, seems like both this one and #24 are related.
I can not apply the dev tools enhancer at ModuleEnhancer as the consumer can already use devtools while creating the store. Also unfortunately we can not apply the dev tools compose twice :(.

I am thinking that I should revert my change to create moduleEnhancer, and just add parameter to configureStore which accepts enhancers that I can compose. @geminiyellow will that satisfy your scenario?

[winklerp]

Hi @navneet-g, I made a quick fix in my repo and added the dev tools composer in moduleEnhancer. This works for me. I think if somebody uses redux-dynamic-modules adding the dev tools compose twice shouldn't be much of an issue, because you have to adjust your setup (configure store, ...) anyway. Maybe we could pass in the composer to moduleEnhancer as a parameter (with default = regular composer)?

What didn't work was to compose moduleEnhancer in redux createstore with the dev tools composer. In that case my module reducers weren't called anymore.

Actually I like the change with moduleEnhancer because we now can use the standard redux createstore method. What seems harder to solve with moduleEnhancer in my opinion is the issue in #24 when passing combined rootreducers to redux createstore. In that case the chained reducers get messed up and undefined keys without reducers are in the store.

[abettadapur]

Can we assert against passing reducers if you have used moduleEnhancer? It's not ideal, but it seems like that should work.

I think we should try to make the enhancer concept work unless it really cannot

[winklerp]

Passing root reducers is not really necessary when using moduleEnhancers because you can pass the reducers with initial modules. That's how I did and it works fine. Maybe just a little note somewhere in the docs.

I agree - the module enhancer concept is good and we should make it work. I like it because we can use the standard redux createStore method.

[navneet-g]

@abettadapur @winklerp I also like moduleEnhancer concept that is why I made the breaking changes :), but it is giving us a lot more trouble then it is helping. I tried few things to make it work with redux dev tools and fixing the issue in #24 but not having any success. I am open to a Pull Request if you have any suggestions.

[winklerp]

I just added these lines in moduleEnhancer.ts and redux devtools work fine:

let composeEnhancers = compose;

if (process.env.NODE_ENV === "development") {
   composeEnhancers = window["REDUX_DEVTOOLS_EXTENSION_COMPOSE"] || compose;
}

We could also pass in composer as optional parameter to moduleEnhancer. Default value = Redux composer. I would appreciate not remove module enhancer. I also see not big deal with issue #24 because as I wrote before passing rootreducers with the initial modules works.

[navneet-g]

Thanks @winklerp ... I already tried that, if we allow consumers to use redux store, I want to function it out of the box. The above will cause three non standard practices

1. Consumers cannot use root reducers, instead need to create aux module
2. They need to pass an overridden compose, if they want to handle the dev mode
3. They need to make sure that moduleEnhancer is on the left side of applyMiddleware if any.

It is going to be hard to explain and might cause bugs. I prefer removing moduleEnhancer and just exposing createStore, as now the interface is pretty clean and easy to understand.

It will be easier for us to test redux-dynamic-modules.

@abettadapur what you suggest.

[abettadapur]

@navneet-g
I see, the main issue is that we have to compose the additional middleware, correct?

I kind of agree now, maybe we should just go back to createStore from the ModuleStore, and allow for enhancers to be passed in correctly. Points 1 and 3 are kind of an issue

[geminiyellow]

woo, breaking change again,
so, now there is no root reducers, right?
if i want to use this lib, i have to rewrite my redux entrance.

[winklerp]

@navneet-g
I agree: the interface of createStore is more clear and it works out of the box. Actually having a Redux-Dynamic-Modules createStore method is not a big deal, as long as all options for configuring the store are exposed.

@geminiyellow
For having rootreducers I created modules, which are initially loaded (passed to createStore method as initial modules).

[geminiyellow]

@winklerp yep, i knew i can put root reducers into a root module,
but that is what i want to ask, in old project,

if i want to use this lib, i have to rewrite my redux entrance.

not just put integral module in some place.

[ghost]

@winklerp and @geminiyellow thanks for your patience, help and feedback while we get it sorted.
I hate breaking changes myself, and this time I guess I got too carried forward while pushing the mouldEnhancer without thinking through all fallouts from that. I apologize again for that.

In terms of @geminiyellow question about using it in old project, I hope there are not too many places where you have redux entrance and it is easier to use.

We have not pushed the update to npm yet. If you have any alternate implementation please share a PR and we can discuss.
We will increase the major version so you don't have to upgrade if you are happy with what you have.

[geminiyellow]

hi @navneetg-msft , i like the moduleEnhancer but i also know #28 (comment) and sorry i dont have a good idea how to solve it now.

PS:

1 point in @navneet-g 's comment,

They need to make sure that moduleEnhancer is on the left side of applyMiddleware if any.

i dont think that is the package should do, we just need to put the notify in docs.

and here:

https://github.com/Microsoft/redux-dynamic-modules/blob/1ee66ccfaca7a49e0fdb0bb93bffe8f002fb1f64/packages/redux-dynamic-modules/src/ModuleStore.ts#L43-L47

how to change the middleware enhancer order if i want to put it in the first of all.
no way, right. that is same They need to make sure that applyMiddleware is on the right side

[geminiyellow]

about this, They need to pass an overridden compose, if they want to handle the dev mode
why dont you create a options param?
i dont think the lib should do this too:

https://github.com/Microsoft/redux-dynamic-modules/blob/1ee66ccfaca7a49e0fdb0bb93bffe8f002fb1f64/packages/redux-dynamic-modules/src/ModuleStore.ts#L38-L40