Angular async pipe not triggering change detection from NgOnChange

Question

I came across such code. The problem was that progress bar was not disappearing after selecting item that already was in cache (when api call inside cache was made it works fine). What I was able to came up with was that change detection was not being run after executing operation in tap. Can someone explain to me why ?

@Component({
    selector: 'app-item',

    templateUrl: `
       <app-progress-bar
          [isDisplayed]="isProgressBar"
       ></app-progress-bar> 
       <app-item-content
          [item]="item$ | async"
       ></app-item-content>`,

    changeDetection: ChangeDetectionStrategy.OnPush
})
export class ItemComponent {
    @Input()
    set currentItemId(currentItemId: string) {
        if (currentItemId) {
            this.isProgressBar = true;
            this.item$ = this.cache.get(currentItemId).pipe(
                tap(() => {
                    this.isProgressBar = false;
                    //adding this.changeDetector.detectChanges(); here makes it work
                })
            );
        } else {
            this.isProgressBar = false;
            this.item$ = of(null);
        }
    }
    item$: Observable<ItemDto>;
    isProgressBar = false;

    constructor(
        private cache: Cache,
        private changeDetector: ChangeDetectorRef
    ) {}
}

Cache is storing items in private _data: Map<string, BehaviorSubject<ItemDto>>; and is filtering out initial null emit

also changing

<app-progress-bar
   [isDisplayed]="isProgressBar"
></app-progress-bar>

to

<app-progress-bar
   *ngIf="isProgressBar"
></app-progress-bar>

makes it work without manually triggering change detection, why ?

Cache:

export class Cache {
private data: Map<string, BehaviorSubject<ItemDto>>;

get(id: string): Observable<ItemDto> {
   if (!this.data.has(id)) {
      this.data.set(id, new BehaviorSubject<ItemDto>(null));
   }
   return this.data.get(id).asObservable().pipe(
      tap(d => {
         if (!d) {
            this.load(id);
         }
      }),
      filter(v => v !== null)
   );
}


private load(id: string) {
   this.api.get(id).take(1).subscribe(d => this.data.get(id).next(d));
}

Edit:

So I figured: tap is being run as an async operation that is why it is being executed after change detection has already run on component. Something like this:

this.isProgressBar = true;
change detection run
tap(this.isProgressBar = false;)

But I was fiddling with it and made something like this:

templateUrl: `
           <app-progress-bar
              [isDisplayed]="isProgressBar$ | async"
           ></app-progress-bar> 
           <app-item-content
              [item]="item$ | async"
           ></app-item-content>`,

        changeDetection: ChangeDetectionStrategy.OnPush
    })
    export class ItemComponent {
        @Input()
        set currentItemId(currentItemId: string) {
            if (currentItemId) {
                this.itemService.setProgressBar(true);
                this.item$ = this.cache.get(currentItemId).pipe(
                    tap(() => {
                        this.itemService.setProgressBar(false);
                    })
                );
            } else {
                this.itemService.setProgressBar(false);
                this.item$ = of(null);
            }
        }
        item$: Observable<ItemDto>;
        isProgressBar$ = this.itemService.isProgressBar$;

And now I have no clue why after doing operation in tap() change detection is not being run on component, does it have something to do with zones ?

ItemService:

private progressBar = new Subject<boolean>();

    setProgressBar(value: boolean) {
        this.progressBar.next(value);
    }

    get isProgressBar$() {
        return this.progressBar.asObservable();
    }

Your "cache" service return always an observable? -if not use of(value) to return an observable- You must include the case your call failled to make this.isProgressBar=false -use catchError- — Eliseo, Dec 07 '18 at 08:34
yes, cache returns observable. I know errors are not handled but don't mind that, I just want to know why change detection is not being run in this example. — pstr, Dec 07 '18 at 08:46
I guess you would need to compose a small working example to clear this. As for zones, you could inject NgZone and check "onMicrotaskEmpty" life cycle hook to see how the execution flow of different async tasks looks like. — briadeus, Jan 03 '19 at 07:49

score 2 · Answer 1 · edited Aug 31 '21 at 17:30

2

For me there's two main issues with your code :

Your cache probably doesn't emit new values (which I don't know because you didn't provide the implementation of it), meaning the async pipe doesn't get triggered,
because you detect onPush, your view is not being refreshed by anything : only the methods/properties you touch are being updated. item$ not being related to your progress bar, you don't see it being updated unles you use detectChanges (which triggers a component change detection).

edited Aug 31 '21 at 17:30

Rafi Henig

5,950
2
16
36

answered Dec 07 '18 at 08:44

Included cache in question. I don't fully understand what you meant in second * but app-item-content is being refreshed while app-progress-bar is not – pstr Dec 07 '18 at 09:23
1

I mean that with `onPush` mode, `async` pipes trigger change detections, but variables value changes don't. Since you use a primitive value (boolean), your changes aren't being detected, resulting in the issue. [This tutorial](https://netbasal.com/a-comprehensive-guide-to-angular-onpush-change-detection-strategy-5bac493074a4) will explain it better than I do ! – Dec 07 '18 at 09:31
Yes I know but, `this tap(() => { this.isProgressBar = false; })` is inside setter for input so change detection its being triggered after `@Input currentItemId` changes – pstr Dec 07 '18 at 10:14
That's the thing, your input doesn't change ! That was in fact my first point. And also, even if the input changes, I think that only the `async` will change, unless you trigger manually the change detection. If you want to try it out, try converting your `isProgressBar` to an observable and use `async` with it. – Dec 07 '18 at 11:35
The input is changing, and change of input is triggering change detection. Scenario was like this - provide `currentItemId` that is already present in cache. So there was input change. You can read what I found in edit. I also tried what you suggested with observable, de facto it was like this from the beginning because other components can light up progress bar also. – pstr Dec 07 '18 at 12:11

Angular async pipe not triggering change detection from NgOnChange

1 Answers1