🐞[BUG]: NgxsReduxDevtoolsPluginModule logs actions in wrong order when dispatching actions from async actions

[perjansson]

Versions

* ngxs: 2.0.0-rc.18
* @angular/core: 5.2.1

Repro steps

When dispatching RequestSearchOptions still RequestSearchOptionsSuccess logs before RequestSearchOptions in ReduxDevTools.

@Action(RequestSearchOptions)
requestSearchOptions({ getState, dispatch }: StateContext<SearchStateModel>) {
	const fetchSearchOptions = async () => {
		const searchOptionsResponse = await provider
			.getOptions()
			.first()
			.toPromise()

		dispatch(new RequestSearchOptionsSuccess(searchOptionsResponse))
	}

	fetchSearchOptions()
}

Observed behavior

NgxsReduxDevtoolsPluginModule logs actions in wrong order when dispatching actions from async actions.

[RobCannon]

I am seeing that too. I have a different pattern for my calls:

 @Action(LoadAllQuotes)
  async loadAllQuotesAction({ getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>) {
    patchState({ quotes: [], selectedQuote: null, busy: true });
    const data = await this.service.getQuotes().toPromise();
    return dispatch(new LoadQuotesSuccess(data));
  }

  @Action(LoadQuotesSuccess)
  async loadQuotesSuccessAction(
    { getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>,
    { payload }: LoadQuotesSuccess
  ) {
    patchState({ quotes: payload, busy: false });
  }

I was using observables, but when I upgraded from ngsx 1.x to 2.0 RC20, the observables stopped working. I debugged and saw that the underlying line that calls HttpClient get was being hit, but the network call was never attempted. After hours, I tried the async/await pattern and the app works correctly (and I really like how the code looks now!), but paired actions in the store show up reversed in the Dev Tools.

[trevor-hackett]

If you wrap dispatch() with setTimeout() it'll be in the correct order. But that shouldn't need to be required.

[RobCannon]

Yes, I just confirmed that. I think the entries into DevTools are being completed when the @action function completes and not when patchState or setState are called.

The sequence seems to be this:

1. First Action starts
2. First Action modifies state
3. First Action dispatches Second Action
4. Second Action modifies state
5. Second Action function completes and returns to First Action
6. Second Action added to DevTools
7. First Action function completes
8. First Action added to DevTools

[deebloo]

@RobCannon can you try with the latest version and confirm if you are still seeing the issue?

[trevor-hackett]

Reproduce: https://stackblitz.com/edit/ngxs-simple-luec15

[amcdnl]

Ok, reading through this and I can confirm this is working as designed right now (now im not saying thats right but its how it is).

So heres the gist:

• Plugins run in a middleware chain, plugin -> plugin -> action
• All actions return an observable (whether you do or not)
• When a action's observable completes, the devtools plugin listens and sends the action
• The actions operate autonomous of other actions, so if i were to fire 3 actions and the 2nd one had a ajax call in it, 1 and 3 would come back first since they completed.

We need to wait for completion before we send the action to the devtools. We also don't want to block any other actions from dispatching since one could be long running. It would also be incorrect to show them in sequential order since they actually didn't run that way. They might have been dispatched that way but not actually executed in that flow.

[RobCannon]

@deebloo I am still seeing states in the "reverse" order after upgrading to @ngxs/store@2.0.0.

I think the issue is with the semantics of dispatch(). It looks like dispatch immediately executes the supplied Action. I would expect that dispatch would add the Action to a queue and once the current Action completes, ngxs would see if there is anything left in the queue.

I tried to look at the source code to see if that is true, but I got lost trying to track the Observables. I can use Observables, but I don't totally grok them enough where I can figure out what is going on in someone else code.

I was having troubles with Observable based actions, but now that the code is out of RC, I will give that a try again and see if that changes anything.

[RobCannon]

I tried Observables again and that is working now, but the DevTool is still showing actions in reverse order.

Here are the two version of code that I am trying:

Observable version:

  @Action(LoadAllQuotes)
  loadAllQuotesAction({ getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>) {
    patchState({ quotes: [], selectedQuote: null, busy: true });
    return this.service.getQuotes().pipe(map(data => dispatch(new LoadQuotesSuccess(data))));
  }

  @Action(LoadQuotesSuccess)
  loadQuotesSuccessAction(
    { getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>,
    { payload }: LoadQuotesSuccess
  ) {
    patchState({ quotes: payload, busy: false });
  }

Async version:

  @Action(LoadAllQuotes)
  async loadAllQuotesAction({ getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>) {
    patchState({ quotes: [], selectedQuote: null, busy: true });
    const data = await this.service.getQuotes().toPromise();
    return dispatch(new LoadQuotesSuccess(data));
  }

  @Action(LoadQuotesSuccess)
  loadQuotesSuccessAction(
    { getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>,
    { payload }: LoadQuotesSuccess
  ) {
    patchState({ quotes: payload, busy: false });
  }

[amcdnl]

This is similar to #213

[amcdnl]

Should be fixed in 3.0

[foralobo]

Hi, I have the version 3.0.1 and also I have the problem. Have You fix?

[amcdnl]

@markwhitfeld - shouldn't the ordered actions fixed this?

[markwhitfeld]

@amcdnl In theory it could have fixed it but I think we made an assumption here about the cause.
I can confirm that the issue is still happening with 3.0.1.
I forked the repo above and updated the deps (love the update all deps to latest button in stackblitz! ):
https://stackblitz.com/edit/ngxs-simple-issue-139-repo?file=app/app.state.ts
I think that we should reopen this issue.

[markwhitfeld]

I had a look at the code here and it sends the info to the redux dev tools when the action has completed. This makes sense because the dev tools requires the new state as part of an action's payload.

By design in NGXS the parent action only completes when the child action completes, so because the child action is completing before the parent it is posting its completed state to the dev tools before the parent.

@amcdnl We will need to figure out how to handle this correctly...
There are probably a few ways we could go here. But just a quick thought is that we could reflect an action in multiple states because we have more defined lifecycle stages for actions than redux:

• On Dispatch
• Every time it triggers a state change
• On Completion/Error/Cancel

[Doug-Shannon]

considering this is clearly still happening - can we reopen the issue?

[markwhitfeld]

Re-opening because we need to figure out an approach that makes sense for the redux dev tools.
PS. We are in the process of making our own dev tools that will fit the NGXS paradigms better.
(See https://github.com/ngxs/devtools )

[filipjnc]

Thanks for reopening the issue. While this is not a crucial topic, it is kind of irritating to have the wrong order. This misses an important point about the Action -> ActionSuccess || ActionError pattern, which most of us love using.

May I ask what are your motives behind the decision to create a new devtools plugin? I myself would feel more comfortable to use the same plugin for all state instances I have open, whether ngrx, NGXS or something else. I find Redux DevTools pretty good and I don’t really see a significant ROI of a new plugin (please enlighten me!).
Has anyone investigated whether replicating ngrx’s way is not going to fix this issue?

P.S. Please also update the issue tags.

[markwhitfeld]

@filipjnc There is much richer information that we are able to gather about the actions that we would want to display in the dev tools. Something that the standard redux dev tools won't support. When you see it you will know why ;-) It will be a while before this is ready though, so we do need to look at better options of displaying info in the redux dev tools.

Ngxs's approach allows for a bit more flexibility than the typical redux style frameworks and as a result some of the paradigms don't fit nicely into what the redux dev tools expect. Actions dispatched as a part of a parent action are considered to be children and as a result the parent action only completes once the children complete. The completion states are what you are seeing in the dev tools, hence the out of order listing.

[markwhitfeld]

Logging the dispatch only will also give an unexpected result because the state is not changed at dispatch.
This is why I thought about logging everything with their lifecycle stage included. See: #139 (comment)

[headsetsniper]

Any news on this Topic? I want to use ngxs instead of ngrx but this is a major drawback

[Karolis92]

Really hard to track app state when this is happening ;/ Any new on this?

[lalmanzar]

Another issue with the DevTool showing actions in reverse order, is that the state changes are not displayed properly on the parent Action. If the parent action changes a property and the same property is then changed on the child, the DevTool shows no change on the parent.

In this example if the service errors out the DevTools shows:
LoadQuotesFail
LoadAllQuotes
And both have no state changes because the busy property changes to true and then back to false.

@Action(LoadAllQuotes)
loadAllQuotesAction({ getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>) {
  patchState({ busy: true });
  return this.service.getQuotes().pipe(
              map(data => dispatch(new LoadQuotesSuccess(data))),
              catchError(error => of(dispatch(new LoadQuotesFail(error))))
              );
}

@Action(LoadQuotesSuccess)
loadQuotesSuccessAction(
  { getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>,
  { payload }: LoadQuotesSuccess
) {
  patchState({ quotes: payload, busy: false });
}

@Action(LoadQuotesFail)
loadQuotesFailAction(
  { getState, setState, patchState, dispatch }: StateContext<IQuoteStateModel>,
  { payload }: LoadQuotesFail
) {
  patchState({ busy: false });
}

[guilhermegjr]

This issue was solved? I am still getting this behavior.

[splincode]

@guilhermechiara sorry, but the problem is still not solved

[razzeee]

The devtools of ngxs seem to be abandoned. At least from what I could gather from ngxs/devtools#118 and the repo.

So it comes down to fixing it in the plugin I guess? Are people using other ways to visualize their state, that I missed?

[Decat-SimonA]

Is there a workaround ?

I tried this:

If you wrap dispatch() with setTimeout() it'll be in the correct order. But that shouldn't need to be required.

But it doesn't seems to work anymore.

[mdgilene]

Checking for an update on this issue. As mentioned above it seems as though the ngxs/devtools project is abandoned. Would be great to see some kind of resolution or potential workaround on this issue.

[DavidTheProgrammer]

Starting a pretty big project and trying out NGXS as my state management. I'm running into this and wondering if it'll have implications down the road. Thinking of switching to Akita but it seems convoluted... Just want something as simple as this but maintained.

[markwhitfeld]

Hi @DavidTheProgrammer. I can assure you that this project is maintained ;-)
The fundamental issue here is that we are "borrowing" the redux dev tools as a dev tool. Unfortunately the redux dev tools has no concept of 'asynchronous' actions (ones that have a lifecycle beyond 'dispatch'). We send the actions to the dev tool once the actions are complete, and therefore, if the first action's completion is dependent on the second action's completing then they will appear to be out of order, because this is in fact the order of completion.

We had a contributor that started writing a dev tool for us (that would recognise the action lifecycle in ngxs), but they couldn't continue due to changes in their personal life, so it hasn't received much attention since then.
I made a comment here about some approaches that we could take to improve this plugin: #139 (comment)
I will raise it with the team to see what they think.

[DavidTheProgrammer]

Hi @markwhitfeld Thanks a lot for taking the time to respond to my comment with such level of detail, I really appreciate it. I now have a better understanding of the scope of the issue and the limitations of the dev-tools. It's unfortunate the contributor could not continue working on the custom plugin, seeing as that's the best solution for us currently, but I guess life happens and I hope he/she/they is/are alright.. Please keep us updated after discussing with the team; I'd be more than happy to help in any way I can.

[vseiche]

Hi @markwhitfeld. I think the whole action chaining mechanism is flawed. Treating dispatched actions from the action handler as child actions might not be the best approach.
I suggest that dispatching actions within another action should be treated as a new action in the chain, so the current action is completed and the dispatched action will be handled as a next action in the chain.
For the asynchronous methods, the Action attribute could be extended to allow to specify two more actions, one for success and one for error. These actions will be dispatched automatically, if they are specified.
I think this will be easier for developers to chain actions and it will solve the order of the actions in redux-devtools.

[amillernib]

This is still a an outstanding issue in 3.7.2

[Hypnoticbg]

This approach fixes the issue:

When wrapping the new dispatch inside asyncScheduler the dispatch waits for the action to complete and then is executed:

Works like a charm:

@Action(LoadData)
  loadData(ctx: StateContext<SampleStateModel>, action: LoadData): void {
    ctx.patchState({
      loaded: false,
    });
    // service has loaded
    asyncScheduler.schedule(() => ctx.dispatch(new LoadDataSuccess('data')));
    
    // service hasn't loaded
     asyncScheduler.schedule(() => ctx.dispatch(new LoadDataFail('err')));
  }

[Halynsky]

@Hypnoticbg Thx for the workaround.
It works but it is still weird when you need to modify your code to see the correct behavior in the dev tool.
By the way, it's better to use asapScheduler

Here is my example. Maybe will be helpful

  @Action(GetUserList, {cancelUncompleted: true})
  getUserList({patchState, dispatch}: StateContext<UserListStateModel>, action: GetUserList) {
    patchState({loading: LoadingState.LOADING});
    return this.userHttpService.getAll(action.params, action.pageable)
      .pipe(
        tap(items => asapScheduler.schedule(() => dispatch(new GetUserListSuccess(items)))),
        catchError(error => {
          asapScheduler.schedule(() => dispatch(new GetUserListFail(error)))
          return throwError(error);
        })
      )
  }

[Hypnoticbg]

@Hypercubed Thx for the workaround. It works but it is still weird when you need to modify your code to see the correct behavior in the dev tool. By the way, it's better to use asapScheduler

Here is my example. Maybe will be helpful

  @Action(GetUserList, {cancelUncompleted: true})
  getUserList({patchState, dispatch}: StateContext<UserListStateModel>, action: GetUserList) {
    patchState({loading: LoadingState.LOADING});
    return this.userHttpService.getAll(action.params, action.pageable)
      .pipe(
        tap(items => asapScheduler.schedule(() => dispatch(new GetUserListSuccess(items)))),
        catchError(error => {
          asapScheduler.schedule(() => dispatch(new GetUserListFail(error)))
          return throwError(error);
        })
      )
  }

We used to use asapScheduler until RxJS 7.x With the latest upgrade with Angular 14+ and RxJS 7.x this stopped working and to be able to work around it we had to change to asyncScheduler wherever we used it.

Regarding the "weird when you need to modify your code to see the correct behavior", I believe this is happening intentionally because the body of the action decorator is just attached to the operator chain in the observable tree of the state. Adding it directly to the chain you don't have a way to delay it until the action is completed and here the scheduler takes place. If you want to have the content executed after the action it must be provided to the state as a callback function that will be executed on completion of the subscriber (The design of NGXS works differently). The author of the library seems to be avoiding this solution for a reason that I am not aware of, which probably causes other issues down the road.

[Halynsky]

@Hypnoticbg Thx for the explanation. Yea I understand the nature of the issue on the same level as you. Just for me, it's a dramatic change of code just for the dev tool. It has 0 business value. And maybe even can produce some additional performance expenses or even make some troubles in SSR considering that Scheduler most likely uses some underhood timeout.
By the way just tested asapScheduler with "@angular/core": "15.2.0", "rxjs": "7.8.0" and it works fine.

[VovkGeorgy]

@arturovt Hello, in duplicate of this issue you asked for the reproducible example, so can you please check this.
May be you would be able to help, because as you can see issues is still reproduce in latest versions and brings a lot of troubles in debug process.

[Hypnoticbg]

@Halynsky It depends on what you do with the action stack. It doesn't affect the Developer tool directly but affects the action stack as a whole and the Developer tool just read it in the wrong way. In our particular case, we track these actions in the order they come so we can track the user journey and this is a problem if the successful actions come before the loading ones. In this case, it makes sense to make small changes to keep the order correct. Regarding the performance, we have tested both ways with the profiler and haven't noticed any peaks or degradation at all.