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

[WIP] Sane action ordering for sync effects #25

Closed
wants to merge 5 commits into from

Conversation

grzegorz-bielski
Copy link
Owner

@grzegorz-bielski grzegorz-bielski commented Sep 19, 2020

Addresses #21

@GrzegorzKazana I couldn't figure out how to make it work with an observeOn, so instead, I delegated the effects subscription.

It means that no effect will be run if there aren't any subscribers to the state$. This is clearly a breaking change, it might need more tests.

Available from npm at @rxsv/core@2.0.0-alpha.0

@GrzegorzKazana
Copy link
Contributor

Dang, I thought it would be a simpler fix.
I think I got it working using the observeOn. Will try to open a PR with that today, so we can discuss what approach is better.

@GrzegorzKazana
Copy link
Contributor

@grzegorz-bielski please have a look at #26, got it working with observeOn, seems like a cleaner solution to me

@grzegorz-bielski
Copy link
Owner Author

Having to rely on state$ subscription is too big of a drawback. Making every effect async seems like a better alternative.
The core issue is addressed in #26

@grzegorz-bielski grzegorz-bielski deleted the fix-action-order-for-sync-effects branch November 25, 2020 13:07
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.

2 participants