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

Epics miss action emitted after `epicMiddleware.run()` depending on environment #624

Closed
tjfryan opened this issue Mar 19, 2019 · 10 comments · Fixed by #625
Closed

Epics miss action emitted after `epicMiddleware.run()` depending on environment #624

tjfryan opened this issue Mar 19, 2019 · 10 comments · Fixed by #625
Labels

Comments

@tjfryan
Copy link

@tjfryan tjfryan commented Mar 19, 2019

What is the current behavior?
Depending on the environment when a store gets created and epicMiddleware.run() gets called, an action dispatched immediately after will get missed by the epics. This seems to happen when epicMiddleware.run() occurs somewhere as a side effect of an observable using the queue scheduler (possibly others)

If the current behavior is a bug, please provide the steps to reproduce and a minimal demo of the problem using JSBin, StackBlitz, or similar.

https://stackblitz.com/edit/redux-observable-playground-emg4do?file=index.js

What is the expected behavior?
'TEST ACTION' will always get emitted to the epics regardless of where it's dispatched, as long as it's after epicMiddleware.run(). The programmer initializing the redux store and epic middleware should not have to be conscious of the code calling surrounding it.

Which versions of redux-observable, and which browser and OS are affected by this issue? Did this work in previous versions of redux-observable?
Happens with latest version. Bug found in an electron app, reproduced in chromium (stackblitz)

@jayphelps

This comment has been minimized.

Copy link
Member

@jayphelps jayphelps commented Mar 21, 2019

Ah yes. Interesting find. This bug happens only inside a running task that was scheduled by the queueScheduler, it shouldn't ever happen any other time. What's happening is that the root epic is not yet subscribed because that happens on the queueScheduler and because we're already inside a queueScheduler task that is still running, it waits until it returns before proceeding.

I'll see what I can do to mitigate this, perhaps removing the subscribeOn(queueScheduler) by moving some other things around. Right now this is necessary.

Is it possible to describe how you hit this? That is, your real world use case that you came across? It might be hard to describe, but anything is helpful! Super appreciate that you first started with a minimum reproduction, was very helpful to confirm!

@BlackHole1

This comment has been minimized.

Copy link

@BlackHole1 BlackHole1 commented Mar 21, 2019

Is there a practical difference between using queues here and not using them?

@jayphelps

This comment has been minimized.

Copy link
Member

@jayphelps jayphelps commented Mar 21, 2019

@BlackHole1 using them internally by redux-observable? Yep. It was added in 1.0.0 to support some recurring complaints people had, mostly related missing actions emitted by epics at startup or the order of output actions not matching their intuitions. e.g. this case and this one

@jayphelps

This comment has been minimized.

Copy link
Member

@jayphelps jayphelps commented Mar 21, 2019

I believe I have a working fix here #625

@BlackHole1

This comment has been minimized.

Copy link

@BlackHole1 BlackHole1 commented Mar 21, 2019

Thank you, I think I understand.
Regarding the place you fixed, is it possible to use AsyncScheduler directly?
It seems to be the same as queueScheduler.constructor

@jayphelps

This comment has been minimized.

Copy link
Member

@jayphelps jayphelps commented Mar 21, 2019

@BlackHole1 One thing: it uses QueueScheduler rather than AsyncScheduler (in case that wasn't a typo hehe.) But unfortunately nope, QueueScheduler--capital Q, the constructor, not queueScheduler the singleton--is not publicly exported by RxJS currently. It was something we debated, but at the time we didn't think there was a valid use case for exporting the constructors.

@BlackHole1

This comment has been minimized.

Copy link

@BlackHole1 BlackHole1 commented Mar 21, 2019

Okay, I understand. Thank you for your answer

@tjfryan

This comment has been minimized.

Copy link
Author

@tjfryan tjfryan commented Mar 25, 2019

@jayphelps I was trying to upgrade the redux-observable version 1.0.0 on a codebase that used redux in unconventional ways in some of the more legacy parts of it, namely, having certain objects manage their state and side effects with it while interacting with other reduxy objects (instead of the suggested 1-per-app).

jayphelps added a commit that referenced this issue Mar 26, 2019
…else's RxJS code, fixes #624 (#625)
@jayphelps

This comment has been minimized.

Copy link
Member

@jayphelps jayphelps commented Mar 26, 2019

Fix landed in v1.1.0 could you confirm all is good now?

@tjfryan

This comment has been minimized.

Copy link
Author

@tjfryan tjfryan commented Mar 26, 2019

Yup! Thanks a ton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.