search

salsita / redux-side-effects

Redux toolset for keeping all the side effects inside your reducers while maintaining their purity.
MIT License
180 stars 9 forks source link

dispatching from external store (like devtools) should not execute effects #15

Closed minedeljkovic closed 8 years ago

minedeljkovic commented 8 years ago

This should fix #14 It is little intimate with how createStore is using its dispatch internally, but I could not find a better way to do it.

Please note that replaceReducer wrapper function no longer has to be closed, so it can be returned to be simple module function.

After I did this I saw that you made this PR to try to make ground for less hacky solution :) This one can serve temporarily or if that PR is not accepted.

tomkis commented 8 years ago

You PR is much appreciated!

It looks like https://github.com/gaearon/redux-devtools/pull/263 will probably get merged, let's wait couple days what's Dan attitude about that.

If you insist on using that currently, please use your own local copy for development. If the PR will not get merged we will extend this because we would really need some unit tests.

Thanks again!

tomkis commented 8 years ago

@minedeljkovic merged & did couple changes, I've published alpha version on npm, could you please check if that's correct? After it's tested we'll do regular distribution publishing.

You can install it via:

npm install redux-side-effects@1.1-alpha

minedeljkovic commented 8 years ago

@tomkis1 I like your changes.

Unfortunately, this is not still perfect. :( I came up with one more scenario that is executing effects when devtools lifted store is dispatching and this time I am afraid there is no easy way out (or I am not seeing one).

When persistState enhancer from devtools is used, action from persisted session are being replayed during createStore call, i.e. when we assume effects should be executed because of regular store initial dispatch.

I will try to look for some solutions, but it I am a bit pessimistic.

tomkis commented 8 years ago

Actually this must have been bug even in previous implementation, or am I missing something here?

minedeljkovic commented 8 years ago

Sure, nothing is broken with new implementation.

I just happen to test it further, once common devtools operation was fixed, and discovered this additional issue.

tomkis commented 8 years ago

Would be worth investigating how redux-loop is solving this. I doubt this is somehow solvable, we should rather think how redux-devtools can be improved to allow this.