-
Notifications
You must be signed in to change notification settings - Fork 406
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
Action stream consolidation #324
Changes from 12 commits
f0a75ee
abdd19a
2f0099c
c891d45
fb6c891
12aaad1
1cbd1c0
083ad11
94eabb1
8f0c382
e8ed37e
4216345
8da51af
6f16968
82af6f8
b27c159
6c236bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
import { Injectable, ErrorHandler } from '@angular/core'; | ||
import { Observable, of, forkJoin, empty, Subject } from 'rxjs'; | ||
import { catchError, shareReplay, filter, exhaustMap, take } from 'rxjs/operators'; | ||
|
||
import { compose } from './compose'; | ||
import { InternalActions, ActionStatus, ActionContext } from './actions-stream'; | ||
import { StateStream } from './state-stream'; | ||
import { PluginManager } from './plugin-manager'; | ||
|
||
/** | ||
* Internal Action result stream that is emitted when an action is completed. | ||
* This is used as a method of returning the action result to the dispatcher | ||
* for the observable returned by the dispatch(...) call. | ||
* The dispatcher then asynchronously pushes the result from this stream onto the main action stream as a result. | ||
*/ | ||
@Injectable() | ||
export class InternalDispatchedActionResults extends Subject<ActionContext> {} | ||
|
||
@Injectable() | ||
export class InternalDispatcher { | ||
constructor( | ||
private _errorHandler: ErrorHandler, | ||
private _actions: InternalActions, | ||
private _actionResults: InternalDispatchedActionResults, | ||
private _pluginManager: PluginManager, | ||
private _stateStream: StateStream | ||
) {} | ||
|
||
/** | ||
* Dispatches event(s). | ||
*/ | ||
dispatch(event: any | any[]): Observable<any> { | ||
let result: Observable<any>; | ||
|
||
if (Array.isArray(event)) { | ||
result = forkJoin(event.map(a => this._dispatch(a))); | ||
} else { | ||
result = this._dispatch(event); | ||
} | ||
|
||
result.pipe( | ||
catchError(err => { | ||
// handle error through angular error system | ||
this._errorHandler.handleError(err); | ||
return of(err); | ||
}) | ||
); | ||
|
||
result.subscribe(); | ||
|
||
return result; | ||
} | ||
|
||
private _dispatch(action: any): Observable<any> { | ||
const prevState = this._stateStream.getValue(); | ||
const plugins = this._pluginManager.plugins; | ||
|
||
return compose([ | ||
...plugins, | ||
(nextState, nextAction) => { | ||
if (nextState !== prevState) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the complexity of this function now, we should break it out into its own method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
this._stateStream.next(nextState); | ||
} | ||
|
||
const actionResult$ = this._actionResults.pipe( | ||
filter((ctx: ActionContext) => { | ||
return ctx.action === nextAction && ctx.status !== ActionStatus.Dispatched; | ||
}), | ||
take(1), | ||
shareReplay() | ||
); | ||
|
||
actionResult$.subscribe(ctx => { | ||
this._actions.next(ctx); | ||
}); | ||
this._actions.next({ action: nextAction, status: ActionStatus.Dispatched }); | ||
|
||
return actionResult$ | ||
.pipe( | ||
exhaustMap((ctx: ActionContext) => { | ||
switch (ctx.status) { | ||
case ActionStatus.Completed: | ||
return of(this._stateStream.getValue()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the subscribe result should not return a value. The reason is was before was the value is needed for the plugins There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amcdnl yeah we would have to change plugins. Which I am certainly open to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that I will have to leave it like this for now so that I don't break the plugins. |
||
case ActionStatus.Errored: | ||
return of(this._stateStream.getValue()); // This was previously the error value | ||
// I think that this should rather | ||
// return throwError(new Error('the error goes here')) | ||
default: | ||
return empty(); | ||
} | ||
}) | ||
) | ||
.pipe(shareReplay()); | ||
} | ||
])(prevState, action) as Observable<any>; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deebloo I think you might like this.