Change detection stopped working in production

[zygimantas]

Versions

* ngxs: 3.1.1/3.1.2
* @angular/core: 6.0.3

Repro steps

• In smart component, add @Select(ZooState.panda) public panda$!: Observable<Panda>;
• In the same component, create subscription: this.panda$.subscribe(a => this.panda = a);

Observed behavior

When this.panda is changed, change detection does not happen in production environment and UI is not updated (*ngIf="panda").

Desired behavior

Change detection should happen.

Mention any other details that might be useful (optional)

It works as expected in 3.0.1 - all environments, 3.1.1/3.1.2 - development environment. There is NO ChangeDetectionStrategy.OnPush set on component.

[ufoscout]

I have the same issue. The only workaround I found is to manually call ApplicationRef.tick() on each effect. For example:

@State<HomeStateModel>({
  name: 'home',
  defaults: new HomeStateModel(),
})
export class HomeState {

  constructor(private service: MyHttpService, private ref: ApplicationRef) {
  }

  @Action(events.CreateCluster)
  createCluster({ getState, setState }: StateContext<HomeStateModel>, { payload }: events.CreateCluster) {
    return this.service.doSomething(payload).pipe(
      tap(nodes => {
        const state = getState();
        setState({
          ...state,
          nodes: nodes
        });
        this.ref.tick(); // FORCE A REFRESH HERE
      }
      ),
      catchError(err => {
        this.ref.tick(); // FORCE A REFRESH HERE
        return of("error message")
      })
    )
  }

[amcdnl]

1) I don’t understand why you would make a subscribe like that vs just use a async pipe 2) this is introduced because of zone issue. We need to return select subscribe to the zone. I’ll make a patch shortly

[zygimantas]

1. Because this.panda is used in button click handling/hammerjs/ngOnChanges methods in component.

[amcdnl]

Gotcha, thanks for clarification.

[amcdnl]

should be fixed in 3.1.3

[ufoscout]

@amcdnl The error is still reproducible in 3.1.3.

This is the bad behavior that we have:

• step 1: trigger an action that changes the state
• step 2: expects the UI to display the updated data, but it does not happen
• step 3: if we click anywhere on the screen, then the UI is updated and the new data is visible

Everything works as expected with 3.0.1.

[amcdnl]

Can you make a stackblitz?

[zygimantas]

Just letting you know that my issue was solved with 3.1.3. Thanks!

[ufoscout]

@amcdnl You can find a repro here: https://github.com/ufoscout/WeWeb

Steps to reproduce:

• clone the repo
• enter the "frontend/Angular-Typescript/" folder (ignore all the rest)
• build it: npm install
• run it: npm start
• browse to localhost:4200
• In the navbar there is a language selector, change the language from "it" to "en"

Wrong behavior: the UI still displays "Ciao Mondo" (Italian) instead of "Hello World" (English). If you click anywhere on the screen, then "Hello World" is displayed.

If you downgrade from "@ngxs/store": "^3.1.3" to "@ngxs/store": "3.0.1" then the issue disappears and everything works as expected.

[amcdnl]

@ufoscout - I looked over your application, I believe what you are doing is a anti-pattern and it makes sense why its not working for you.

  @Action(events.SetLanguage)
  setToken({ getState, setState }: StateContext<CommonStateModel>, { payload }: events.SetLanguage) {
    const state = getState();
    setState({
      ...state,
      language: payload.language,
    });
    this.translate.use(payload.language);
  }

https://github.com/ufoscout/WeWeb/blob/master/frontend/Angular-Typescript/src/app/module/common/common.state.ts#L31

Here you are setting your translator in your state. This should not happen here. Only state modifications should happen in your state. I would recommend using action subscriptions to perform this.

Here is a example where I use a action subscription to log a user out and disconnect them from the websocket.

    this._actions.pipe(ofActionDispatched(Logout)).subscribe(() => {
      this.svc.disconnect();
      this.router.navigate(['login']);
  });

I would recommend you subscribing to that event in the action stream and then switching that there. Let me know if this works for you please.

[ufoscout]

@amcdnl yes, the bug is still present in 3.1.3. Please check my previous comment in which I provided a reproducer. There is, however, a difference between the behavior of 3.1.2 and 3.1.3; in fact:

• with 3.1.2 the UI is never refreshed after a change in the state
• with 3.1.3 the UI is refreshed after a click on the screen (please note that the click is required only after the first state change, for subsequent changes the UI is refreshed as expected with no need to click).

[amcdnl]

@ufoscout - Check my updated answer.

[jhonland]

I have a similar problem in my own project, I tried to debug using the Actions handler from the Actions import form ngxs/store, I am subscribing in the component and using the pipe ofActionDispatched() in that subscribe I console.log the value, the first time that the component is instantiated the UI gets updated, but for some reason after a navigate to other route and then return to the initial route and try again, the same ofActionDispatched subscription function console.logs the value but the UI wont update unless I do as @ufoscout says, to use tick() inside the subscription function. Side note I am subscribed to ofActionSuccessful over the same action and after navigating to other route and returning to the initial one, it Updates the console.log value as expected and the UI gets updated also, with no use of Tick() method!! I am using 3.1.3, have not tried downgrading.

[jhonland]

@amcdnl I just tested with version 3.0.1 and it worked out.

[juracy]

I think I have the same issue with mat-dialog (@angular/material): https://ngxs-mat-dialog-issue.stackblitz.io

And It worked perfectly with version 3.0.1

Using dispatch button, the dialog is showed, but the buttons cancel and ok, don't work correctly ... The direct button shows same dialog without using @ngxs, and everything works...

[amcdnl]

@juracy - Your doing the same thing as him, you have code like:

  @Action(ConfirmAsk)
  ask(ctx: StateContext<ConfirmModelState>, action: ConfirmAsk) {
    this.uiAsk(action.message)
    ctx.setState({message: action.message})
  }

  uiAsk(message: string) {
    this.dialog.open(ConfirmDialogComponent, {
      width: '50%',
      data: { message: message },
    })
  }

That function should NOT live in your state. You should use the action stream to subscribe and react. This is actually a good thing this is happening to weed out these anti-patterns.

Unless someone says otherwise, I'm going to close this because it seems like its working for the appropriate cases now.

[juracy]

@amcdnl I also tried with action handlers and I haven't got success anyway...

I've just updated my example with handler, and the same problem remains... https://ngxs-mat-dialog-issue.stackblitz.io

[juracy]

I could manage some workarounds:

• ApplicationRef.tick() in click handlers of buttons
• Call MatDialog.open in ngZone.run, in action handler or in state class, whatever. I have peep in https://github.com/ngxs/store/blob/01c499f1e33a40ffebbf66ffbb46fcadb6b17d9b/packages/router-plugin/src/router.state.ts#L55

[jhonland]

@juracy I have just discovered what is troubling me with my code, the view doesn't gets updated as I said in my comment, 'cause i was trying to change the route with the Router from angular, directly from the app state (in my case after dispatching a Login action that was successful) I just replace it with the Navigate action, provided by '@ngxs/router-plugin' and problem solved!. keep in note I was trying to display statuses like 'pending' and 'error' that lives not in the state (as it could) but rather lives in the component, a gets updated by subscribing to it 'this.store.dispatch(new Login).subscribe(success => {}, error => {})'.

Conclusion, I think #376 was needed in some extend. As @amcdnl said

"Only state modifications should happen in your state"

maybe we should update the Docs to be a more cleare aboute what is permited to do inside the app state action function. For last, point to this ((https://github.com/ngxs/ngxs-examples)) example at the README file in the @ngxs/store Repo, because it help me allot to understand what was happening.

onSubmit() {
    if (this.loginForm.enable) {
      if (this.loginForm.valid) {
        this.pending = true;
        this.subscription = this.store.dispatch(new fromAuthActions.Login( this.loginForm.value )).subscribe(
          () => { this.pending = false; },
          error => { this.pending = false; this.loginForm.markAsPristine(); this.error = error.error; }
        );
      }
    }
  }

<div class="errors" *ngIf="loginForm.pristine && error">
          Ocurrio un error
</div>
<div class="loading" *ngIf="pending">LOADING...</div>

@Action(fromAuthAction.Login)
  login(ctx: StateContext<AuthStateModel>, { payload }: fromAuthAction.Login) {
    return this.authService.login(payload).pipe(
      tap(user => {
        ctx.patchState({ user, isLoggedin: true });
        this.router.navigate(['/']);      <=== BAD
      })
    );
  }

@Action(fromAuthAction.Login)
  login(ctx: StateContext<AuthStateModel>, { payload }: fromAuthAction.Login) {
    return this.authService.login(payload).pipe(
      tap(user => {
        ctx.patchState({ user, isLoggedin: true });
        ctx.dispatch(new Navigate(['/']));      <=== GOOD
      })
    );
  }

[juracy]

@jhonland like I said, my last example was updated in https://stackblitz.com/edit/ngxs-mat-dialog-issue and I took off all side effects in the state class, even though, the problem remains ... I'm using o action stream now!

And in your example about router, nowadays it's working using ngZone.run like you can see in @ngxs/router-plugin: https://github.com/ngxs/store/blob/01c499f1e33a40ffebbf66ffbb46fcadb6b17d9b/packages/router-plugin/src/router.state.ts#L55

I don't believe that is the best approach anyway...

[amcdnl]

@juracy - I looked at your example and it seems to work to me? What am i missing?

Also, is this your real use case? It feels a bit odd to put a confirm state in a global state container like that.

[juracy]

@amcdnl sorry, steps: 1 - Click on dispatch button 2 - Click on Cancel action button in dialog

Expected behavior: log a message in console and close dialog

[ufoscout]

@amcdnl Moving the language selection from the state, as you suggested, solved that particular the issue; nevertheless, there are still use cases that work fine in 3.0.1 but not in 3.1.3. For example, I have a spinner image to be displayed while certain actions are running. in 3.0.1 this works as expected:

  @Action(events.Login)
  login(ctx: StateContext<AuthStateModel>, {payload}: events.Login) {
    this.spinner.show();
    return this.loginService.login(payload).pipe(
      map(response => {
        this.spinner.hide();
        ctx.dispatch(new events.SetAuthData(response));
      }),
      catchError(err => {
        this.spinner.hide();
        return throwError(err);
      })
     );
  }

but in 3.1.3 the spinner is not shown. Moving the spinner from the state to somewhere else is not easy in this case. In fact, I cannot create a global interceptor since I need it only for some actions and create a special subscription to the Action Dispatched and Action Completed for each interested action seems really overcomplex for this simple task. Is there any technical reason to not re-enable the same behavior of 3.0.1?

[juracy]

@ufoscout my two cents: 1 - Move the spinner interaction to a handler (action stream), that's the best practice 2 - Even though, run interaction with UI within ngZone (at least, it's my current workaround)

[amcdnl]

@ufoscout - The state is not supposed to interact w/ the UI. Hiding the spinner/etc is UI related and since we run state outside of zone it will not fire. Those should be action handlers or driven by state in the UI.

[jhonland]

@ufoscout You should start your spinner from inside your component before dispatching your action, then you should subscribe to that action dispatch an when subscribed its executed (after your action call your login API and return) you tell your spinner to stop. Something like this:

onSubmit() {
    if (this.loginForm.enable) {
      if (this.loginForm.valid) {
        this.pending = true;
        this.subscription = this.store.dispatch(new fromAuthActions.Login( this.loginForm.value )).subscribe(
          () => { this.pending = false; },
          error => { this.pending = false; this.loginForm.markAsPristine(); this.error = error.error; }
        );
      }
    }
  }

where the this.pending would be your spinner methods to start and to stop

[juracy]

@amcdnl I have took off UI code from state and put it in an action handler, but I still need to use ngZone, is that correct?

Did you see my example? https://stackblitz.com/edit/ngxs-mat-dialog-issue

Steps: 1 - Click on dispatch button 2 - Click on cancel button (in dialog)

Expected:

• Close dialog

[markwhitfeld]

@amcdnl I can confirm that there is a bug if you use the action stream. It is because the action stream is not returning in the angular zone. I will submit a fix today.

@juracy See your sample with a temp workaround to get it working: https://stackblitz.com/edit/ngxs-mat-dialog-issue-eo2aqj?file=src/app/confirm/confirm-handler.ts

Also, see a sample repro here of the issue and potential workarounds until it is fixed: https://stackblitz.com/edit/ngxs-actionstream-bug-repro?file=app/app.component.ts

[juracy]

@markwhitfeld thanks! I had already been using ngZone! But I thought it's wrong or at least weird!

[markwhitfeld]

Confirmed fixed with latest dev package: https://stackblitz.com/edit/ngxs-mat-dialog-issue-fixed?file=src/app/confirm/confirm-handler.ts and https://stackblitz.com/edit/ngxs-actionstream-bug-repro-fixed?file=app/app.component.ts

[jeffkenz89]

just want to let you know ngxs 3.14 still has this bug in close button like @juracy bug,after update to 3.14 NGXS/store@dev the bug is fix

[MurhafSousli]

@jeffkenz89 This is not a bug, when you want to update the UI from the state code use the following

this.ngZone.run(() => {

  this.dialog.open(ErrorDialogComponent, {
     width: '250px',
     data: payload.error.message,
     disableClose: true
  });

});