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

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

Merged
merged 1 commit into from
Apr 5, 2016

Conversation

minedeljkovic
Copy link

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
Copy link
Contributor

tomkis commented Apr 2, 2016

You PR is much appreciated!

It looks like reduxjs/redux-devtools#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 tomkis merged commit 1c15ab9 into salsita:master Apr 5, 2016
@tomkis
Copy link
Contributor

tomkis commented Apr 6, 2016

@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
Copy link
Author

@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.

@minedeljkovic minedeljkovic deleted the fix-devtools branch April 6, 2016 15:35
@tomkis
Copy link
Contributor

tomkis commented Apr 18, 2016

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

@minedeljkovic
Copy link
Author

minedeljkovic commented Apr 18, 2016

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
Copy link
Contributor

tomkis commented Apr 18, 2016

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.

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

Successfully merging this pull request may close these issues.

1.0.0 broke time travelling and hot reloading
2 participants