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

1.0.0 broke time travelling and hot reloading #14

Closed
minedeljkovic opened this issue Mar 30, 2016 · 5 comments · Fixed by #15
Closed

1.0.0 broke time travelling and hot reloading #14

minedeljkovic opened this issue Mar 30, 2016 · 5 comments · Fixed by #15
Labels

Comments

@minedeljkovic
Copy link

In version 1.0.0 effects are deferred from enhanced reducers even on dispatch initiated by redux dev tools, during time travelling or reducer hot reloading.
If I remember well, one of main goals of redux-side-effects was not to brake any of dev tools functionality.
Is there a workaround for this, or am I missing something about new implementation?

@tomkis tomkis added the bug label Mar 30, 2016
@tomkis
Copy link
Collaborator

tomkis commented Mar 30, 2016

This indeed must a bug. Let me investigate and get back to you. I didn't realize that this breaking change could potentially break devtools. Definitely top priority now.

@tomkis
Copy link
Collaborator

tomkis commented Mar 31, 2016

Fixed in 1.0.1

@tomkis tomkis closed this as completed Mar 31, 2016
@minedeljkovic
Copy link
Author

This fixed half of the problem - hot reloading. Time traveling still executes effects.

Problem is that devtools liftedStore, during time travelling, dispatches to same enhanced reducer, which then deffer effects to side effects enhanced store (one created by createEffectCapableStore).

I think side effects enhanced store should ignore effects while not dispatching (i.e. while some other store like liftedStore dispatches).
Problem is it is not so easy to check who is dispatching.
Wrapping dispatch is not 100% accurate since original store dispatches once before returning dispatch to be wrapped (https://github.com/reactjs/redux/blob/master/src/createStore.js#L204). I wonder if this was a bug in previous version of redux-side-effects :)

I will try to look more into this. @tomkis1 Do you have any idea, how this could be solved?

@tomkis tomkis reopened this Apr 1, 2016
@tomkis
Copy link
Collaborator

tomkis commented Apr 1, 2016

Oh sorry, didn't realize that you mean time travelling too I checked only the "devtools" part.

I wonder if this was a bug in previous version of redux-side-effects :)

Absolutely! This is the main reason why I made the decision to rewrite it, there was no other option to work it around. I wanted to keep the behaviour consistent but now it's obvious we'll have pretty hard times figuring out how to make time travel working.

@tomkis
Copy link
Collaborator

tomkis commented Apr 3, 2016

@minedeljkovic after @gaearon decided to close the PR (and his decision was right) and me realizing that it does make sense to provide custom initialize function i've decided to go back with wrapped dispatch.

I'll fix this issue by using wrapped version of dispatch instead of executing side effects in every reducer call.

@tomkis tomkis closed this as completed in #15 Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants