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] Awaitable store with async saga support #62

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

[WIP] Awaitable store with async saga support #62

wants to merge 10 commits into from

Conversation

cmeeren
Copy link

@cmeeren cmeeren commented Apr 3, 2017

This is a work in progress. Posting here as a PR so we can discuss. Particularly interested in what @dcolthorp has to say. Regarding #47 (comment), this PR provides a way of registering sagas using an action observable AND awaiting the completion of all actions in the store. See unit tests for details.

Due to necessity I branched this off of my previous branch (PR #60), hence why the commit from that PR also shows up in this PR.

I've just lumped all code together in one file for now.

The distinct features in this PR are:

  • The following two delegates denoting async/blocking sagas:

    • void Saga(action, store)
    • Task AsyncSaga(action, store)

    I'm considering changing this a bit so that they only accept an action parameter, and then have the sagas be returned from a function that accepts the store. Kind of like middleware, where the middleware has access to the store through the function that creates the middleware. Removing the store parameter would make the saga register syntax look cleaner, and also removes the store parameter from sagas that don't need it. Input welcome.

  • Two extension methods on IObservable<T> for registering async and non-async sagas.

  • ObservableActionStore which exposes an IObservable of actions. Can be dropped in favor of a simple extension to Store if we figure out New ActionDispatched event would duplicate StateChanged #63.

  • AwaitableStore which exposes DispatchAsync and also has internally accessible member(s) for keeping track of async operations that DispatchAsync should await (currently this is simply a method IDisposable AsyncOperation() to be used in using statements, as seen in the extension method.)

  • The IObservable<T> extension methods RunsSaga and RunsAsyncSaga

Things to do before this can be merged:

  • Structure: Where to put the classes and interfaces
  • Determine if/how various features should be integrated with existing Redux.NET classes.
  • @GuillaumeSalles must be satisfied with the PR
  • There should ideally be no significant objections from the always helpful @dcolthorp and @lilasquared

@cmeeren cmeeren mentioned this pull request Apr 3, 2017
@cmeeren
Copy link
Author

cmeeren commented Apr 4, 2017

@dcolthorp It should be exception safe now. Calling Dispatch (on any store) or await DispatchAsync() (on AwaitableStore) will 1) bubble up the exception, and 2) make sure that the async task count is decremented.

I've added a ton of tests, check them out to see how it's used. Some tests might be unnecessary due to being pretty much being guaranteed by .NET, but much of it was copy-paste and I didn't stop to think too hard on whether all the tests were strictly needed.

@cmeeren
Copy link
Author

cmeeren commented Apr 4, 2017

I've updated the initial comment with more information.

@dcolthorp Regarding #47 (comment) and your mention of TakeLatest: I tried experimenting, but couldn't find a solution that was simpler than simply letting the sagas themselves handle this. I do that already in a few of my listeners.

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 participant