feat(store): create action stream that shows the action lifecycle #255
Conversation
|
I assume that the @Action(Completed(FeedAnimals))or @Action(onCompletion(FeedAnimals))or @ActionCompleted(FeedAnimals) |
|
Hmmmm. Not sure if we wanna do that or not. Could potentially also be another config option. |
|
Whatever we decide I think it will have to wait till after this based on the way our current dispatch works |
docs/advanced/action-handlers.md
Outdated
|
|
||
| * `ofAction`: triggers both when an action has been dispatched and when it completes | ||
| * `ofActionDispatched`: triggers when an action has been dispatched but NOT when it completes | ||
| * `ofActionCompleted`: triggers when an action has been completed but NOT when it is dispatched |
There was a problem hiding this comment.
We might want a ofActionErrored too. Just a thought.
packages/store/src/state-factory.ts
Outdated
| @@ -117,7 +117,7 @@ export class StateFactory { | |||
| if (result instanceof Observable) { | |||
| result = result.pipe( | |||
| (<ActionOptions>actionMeta.options).cancelUncompleted | |||
| ? takeUntil(actions$.pipe(ofAction(action.constructor))) | |||
| ? takeUntil(actions$.pipe(ofActionDispatched(action.constructor))) | |||
There was a problem hiding this comment.
is action here the class or instance? It looks like instance and if so, this will cause issues if the action dispatched is not a class (think devtools or websocket). We probably need to change the action.constructor to check and return.
There was a problem hiding this comment.
it is counting on an instance yes. we can make this more robust but we should also be safer in the operators as well
There was a problem hiding this comment.
actually I should probably put all of that logic in the operators anyway
| @SkipSelf() | ||
| parent: StateStream | ||
| ) { | ||
| constructor() { |
There was a problem hiding this comment.
If we remove all that, do we event need the constructor now?
There was a problem hiding this comment.
I think something breaks if there is no immediate value
| .pipe(map(() => this._stateStream.getValue())); | ||
| .pipe( | ||
| tap(() => { | ||
| this._actions.next({ action, status: ActionStatus.Completed }); |
There was a problem hiding this comment.
What do you think about making these class and just say: new CompletedAction(action)?
There was a problem hiding this comment.
This would actually add more boilerplate since we would have to create a new class for each type we create
packages/store/src/of-action.ts
Outdated
| return filter((ctx: ActionContext) => { | ||
| const actionType = getActionTypeFromInstance(ctx.action); | ||
|
|
||
| if (status) { |
There was a problem hiding this comment.
I'd prefer:
const type = allowedTypes[actionType] ;
return status ? type && ctx.status === status || type;
docs/advanced/action-handlers.md
Outdated
| * `ofAction`: triggers both when an action has been dispatched and when it completes | ||
| * `ofActionDispatched`: triggers when an action has been dispatched but NOT when it completes | ||
| * `ofActionCompleted`: triggers when an action has been completed but NOT when it is dispatched | ||
|
|
There was a problem hiding this comment.
Need to add this to the changelog too.
|
@markwhitfeld listening via decorators would be super cool but not possible due to AoT and needing to unbind on destroy. see this for more info: https://medium.com/thecodecampus-knowledge/the-easiest-way-to-unsubscribe-from-observables-in-angular-5abde80a5ae3 |
|
@amcdnl I don't think AOT has anything to do with this in particular. I think we could investigate but would def be a separate issue |
|
import { Actions, ofAction, ofActionComplete, ofActionDispatched } from '@ngxs/store';
import { ofActionErrored } from '@ngxs/store/src/of-action'; |
No description provided.