ngIf Expression has changed after it was checked

Question

I will explain my problem with a simplified example since my code is quite lengthy.

I want to display a certain div when either one of isFetching or isDownloading is true.

<div *ngIf="(isFetching || isDownloading)">
  My div element is here
</div>

In the start of my program both of them are true, so the div should be displayed. Below the div from above, I have other elements which are displayed on the view after isFetching is changed to false. (isFetching is changed to false after the fetching from the database has completed.) After isFetching is changed to false and the elements are rendered, after the last element of these is downloaded (it is an image), I then change isDownloading to false.

When both isFetching and isDownloading are false, this is when I want to hide the div element.

However, the code I just explained, gives me this error:

Expression has changed after it was checked. Previous value: 'ngIf: true'. Current value: 'ngIf: false'.

I do not seem to understand why do I get this issue? What is wrong with my logic? How can I achieve the desired result without getting this error?

EDIT:

This code is in the constructor of my class:

firebase.database().ref('/users/' + this.userId).once('value').then(function(snapshot) {
  this.username = (snapshot.val() && snapshot.val().username) || 'Anonymous';
  this.isFetching = false;
});

After isFetching is changed to false. There are other elements in my .html which get rendered. And then this code is called after the last element (which is an image) of those elements is loaded (load)="latestElementLoaded()".

latestElementLoaded(){
   this.isDownloading = false;
}

Can you please add the code that's responsible for setting both `isFetching` and `isDownloading`? — user184994, Jul 29 '18 at 12:43
Possible duplicate of [ngIf - Expression has changed after it was checked](https://stackoverflow.com/questions/43513421/ngif-expression-has-changed-after-it-was-checked) — Rodrigo Ferreira, Jul 29 '18 at 12:48
[This comment](https://github.com/angular/angular/issues/17572#issuecomment-364175055) in the official repo might be helpful — Rodrigo Ferreira, Jul 29 '18 at 12:53
You may want to move that code out of the constructor and into `ngOnInit` — user184994, Jul 29 '18 at 12:53
@RodrigoFerreira, This was quite useful indeed. Since the problem is with the structure of my .html, could you provide me with an advice on how to restructure my code so it is ` single change detection pass from the top down.` I want to display my div when either one of those `isFetching`or `isDownloading` is `true`. Since I cannot do it with an `||` operator, how should I proceed? Should I create another element which is a duplicate of that div and only have `*ngIf="isDownloading"` attached to it? However, I do not think this is the best workaround. — EDJ, Jul 29 '18 at 12:59
did you try @user184994 suggestion? to move it from constructor to ngOnInit? — dAxx_, Jul 29 '18 at 13:01
@user184994, I have just tried it yes. However, it did not fix the error. — EDJ, Jul 29 '18 at 13:01

score 1 · Answer 1 · 2018-07-30T13:43:40.497

You may want to consider an alternative pattern, using observables, which can streamline your code (AngularFire is worth looking into for that):

this.data$ = this.angularFirestore.doc('some/path').valueChanges();

That gets your data as an observable (this is just demonstration code--check the actual syntax). Put this in ngOnInit, not the constructor. No screwing around with .once or snapshots or other boilerplate.

Now, in your template, you can write

<div *ngIf="data$ | async as data; else notFetched">
  Data is {{data | json}}
  <ng-template #notFetched>Data is not fetched yet..wait...</ng-template>
</div>

This eliminates the need for isFetching-like flags altogether.

With regard to your code:

firebase.database().ref('/users/' + this.userId).once('value').then(function(snapshot) {
  this.username = (snapshot.val() && snapshot.val().username) || 'Anonymous';
  this.isFetching = false;
});

I have no idea how this would work at all, since this inside the callback function (function(snapshot)) is probably going to be undefined, giving you a run-time error.

I'm no expert on Angular internals, but I suspect what is happening to cause the error you report is the following sequence of events:

Component is instantiated and constructor runs.
Firebase query kicks off.
ngOnInit is called and change-detected variables examined. isFetching is still true at this point.
Angular does more work building the view and what have you.
Meanwhile, the Firebase query finishes, and 'isFetching` is set to false.
Before finalizing this round of rendering, Angular checks one more time (it does this in test mode) just to make sure nothing has unexpectedly changed. But it has!

This entire problem is caused by putting the firebase query logic in the constructor, as mentioned in a comment. Constructors should be strictly limited to basic initialization. They should not contain business logic, and should definitely not contain asynchronous stuff.

ngIf Expression has changed after it was checked

1 Answers1