bug(mat-selection-list): using objects as value for the mat-list-options does not work

[ArneAdriaenssens]

Is this a regression?

• Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

When using the mat-selection-list with objects to use as values for the nested mat-list-option components, the application freezes and crashes. At first I thought it was a matter of passing a valid function to the compareWith prop but it turns out this function is never called.

During my investigation I noticed that depending on the ChangeDetectionStrategy used, we can prevent a browser crash. When set to Default the browser will crash but using OnPush will prevent this (probably unrelated but useful for debugging).

When using the exact same setup but passing strings as values for the nested mat-list-options components, it works as expected and allows for selecting/deselecting options.

I did not find other bugs that report this behavior, however there is another bug that talks about the compareWith not being called which might be part of the issue.

Reproduction

Steps to reproduce:

1. https://stackblitz.com/edit/components-issue-wp9cof?file=src/app/example-component.html

The first mat-selection-list uses strings and works as expected. The second one fails and does nothing. When changing the change detection to Default it will freeze up your browser.

Expected Behavior

When passing an object as value to the mat-list-option it should allow for selecting and deselecting the corresponding mat-list-option and the mat-selection-list should emit the correct value.

Actual Behavior

When passing an object as value to the mat-list-option the browser freezes or the mat-selection-list does not emit anything

Environment

Angular CLI: 13.0.3
Node: 16.13.0
Package Manager: npm 8.1.0
OS: darwin arm64

Angular: 13.0.2
... animations, common, compiler, core, forms
... platform-browser, platform-browser-dynamic

Package Version

@angular-devkit/architect 0.1300.3
@angular-devkit/build-angular 13.0.3
@angular-devkit/core 13.0.3
@angular-devkit/schematics 13.0.3
@angular/cdk 12.2.13
@angular/cli 13.0.3
@angular/material 12.2.13
@schematics/angular 13.0.3
rxjs 7.4.0
typescript 4.4.4

[crisbeto]

It's because of these two getters:

  get stringOptions() {
    return ['Shoe', 'Boot'];
  }

  get objectOptions() {
    return [{ label: 'hey' }, { label: 'hoi' }];
  }

The getters are returning new array instances every time they're invoked which means that Angular has to re-run change detection because the value has changed. Next time change detection runs, it'll receive brand-new arrays again which can throw the app into an infinite loop. Change detection is based on regular equality checks (===), it doesn't do any deep comparisons between the old and new values. You should be able to fix it by reusing the same objects:

stringOptions = ['Shoe', 'Boot'];
objectOptions = [{ label: 'hey' }, { label: 'hoi' }];

[ArneAdriaenssens]

Yes, indeed. To solve the issue in the provided example avoiding using the getters does indeed solve it, as long as the array doesn't change. However that doesn't explain why it does work for a string array coming from a getter.

After further investigation I think I figured out why it's possible to pass an array of strings coming from a getter but not with an array of objects.

When clicking on a MatListOption, the focus event is emitted. This event triggers change detection causing the getters the be called and passing new arrays to the childs with a different reference. This is the same for the string array as for the object array. However, the difference comes from how *ngFor tries to optimise rendering of its child compnents. In the *ngFor ngDoCheck method, the input array is compared for differences. This uses the default === comparison on every object.
For the string array that's fine, it will detect that there are no differences and it does not destroy/create new child components.
For the object array, it will use the same comparison but because objects are checked by reference it thinks every item has changed. Therefor the *ngFor will destroy all child components and recreate them. But they won't have enough info the detect which option was clicked on before the destruction of previous values

I'm not sure if this can be easily solved as we only receive the focus event before destroying the MatListOption.

[crisbeto]

I'm closing the issue since we can't do much to solve it on our end. Furthermore, using getters like this is an anti-pattern which can lead to unnecessary re-renders and memory leaks.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}