angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.31k stars 6.73k forks source link

cdk-virtual-scroll: newly-rendered list items seem to inherit internal state from the _templateCache #15838

Open michaelsanford opened 5 years ago

michaelsanford commented 5 years ago

What is the expected behavior?

New components rendered by CdkVirtualForOf are pristine by default.

What is the current behavior?

New components created by CdkVirtualForOf within the <cdk-virtual-scroll-viewport> inherit internal state of recycled views by default.

What are the steps to reproduce?

https://stackblitz.com/edit/angular-virtual-scrolling-with-component-state

  1. Click on Data Item 3
  2. Scroll down
  3. Observe Data Item 12 is "active"
  4. Scroll down
  5. Observe Data Item 23 is "active".
  6. Scroll up
  7. Observe Data Item 10 is "active" (a different one, now)

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular CLI: 7.3.8
Node: 11.12.0
OS: win32 x64
Angular: 7.2.12
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, platform-server, router
... service-worker

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.13.6
@angular-devkit/build-angular     0.13.6
@angular-devkit/build-optimizer   0.13.6
@angular-devkit/build-webpack     0.13.6
@angular-devkit/core              7.1.4
@angular-devkit/schematics        7.1.4
@angular/cdk                      7.3.7
@angular/cli                      7.3.8
@angular/pwa                      0.11.4
@ngtools/webpack                  7.3.6
@schematics/angular               7.1.4
@schematics/update                0.13.8
rxjs                              6.4.0
typescript                        3.2.4
webpack                           4.29.0

Is there anything else we should know?

Surely this is the very intent of view recycling, but it seems very strange that the default behaviour of *cdkVirtualFor is very different from *ngFor: my bound parameters changed, the data rendered via the @Input() changed, so why didn't the internal state reset?

It's more than template caching, which most of us would take to mean the HTML: it caches whole components and internal state.

Obviously, setting templateCacheSize: 0 fixes this problem.

Maybe simply highlighting this in the documentation might help, since you have to read between the lines to understand what's happening? Perhaps I'm the only one with this expectation. 🤷‍♂️

ArtemKlots commented 5 years ago

Hi Michael, it is not a defect of Angular. Virtual scroll uses cache to reuse components, but your component is not reusable. Look at my explanation for another issue - you have the same case

https://github.com/angular/components/issues/16330#issuecomment-514591282

Solution can look like this (but state will be lost):

@Component({
  selector: 'hello',
  template: `<h1 [class.active]="active">Data Item {{_item}}</h1>`,
  styles: [
    `.active {background-color: red; color: white}`,
    `h1:hover { background-color: lightblue }`
    ]
})
export class HelloComponent  {
  @Input() set item(item: number) {
    this._item = item;
    this.active = false;
  }

  _item: number;
  active: boolean = false;
  toggle = () => this.active = !this.active;
}
michaelsanford commented 5 years ago

@ArtemKlots While I agree that this is not a defect by virtue of it being a design choice, I still think it might be a good idea to highlight this behaviour in the documentation more clearly, which was the point of this ticket.

I suppose I could do that at this point.

lebesnec commented 4 years ago

I got the same exact "bug" today. The documentation at https://material.angular.io/cdk/scrolling/overview#virtual-scrolling should definitively be updated with information about this behavior.