4

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:

  1. this.isProgressBar = true;
  2. change detection run
  3. 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();
    }
3
  • 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- Commented Dec 7, 2018 at 8: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. Commented Dec 7, 2018 at 8: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. Commented Jan 3, 2019 at 7:49

1 Answer 1

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).

Sign up to request clarification or add additional context in comments.

5 Comments

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
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 will explain it better than I do !
Yes I know but, this tap(() => { this.isProgressBar = false; }) is inside setter for input so change detection its being triggered after @Input currentItemId changes
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.
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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.