fix(list): selection list incorrectly preselecting options without a …

…value in some cases

The way we keep track of the value of an entire selection list is by taking all of the `selected` options and putting their values in an array. When the set of options is swapped out we look through that array to determine whether an option should be preselected. This breaks down if an option doesn't have a value, because we end up with a value array looking like `[undefined]` which will cause the option to be preselected incorrectly. These changes work around the issue by not adding options without a value to the value array.

Fixes angular#17839.

src/material/list/selection-list.spec.ts

@@ -41,6 +41,7 @@ describe('MatSelectionList without forms', () => {
           SelectionListWithIndirectChildOptions,
           SelectionListWithSelectedOptionAndValue,
           SelectionListWithIndirectDescendantLines,
+          SelectionListWithDynamicOptionsWithoutValue,
         ],
       });
 
@@ -612,6 +613,34 @@ describe('MatSelectionList without forms', () => {
       expect(option.classList).toContain('mat-2-line');
     });
 
+    it('should not preselect options without values when the list of options is swapped', () => {
+      const componentFixture = TestBed.createComponent(SelectionListWithDynamicOptionsWithoutValue);
+      componentFixture.detectChanges();
+      listOptions = componentFixture.debugElement.queryAll(By.directive(MatListOption));
+      selectionList = componentFixture.debugElement.query(By.directive(MatSelectionList))!;
+      const list: MatSelectionList = selectionList.componentInstance;
+
+      expect(list.options.map(option => option.selected)).toEqual([false, false]);
+
+      componentFixture.componentInstance.items = [
+        {name: 'One', selected: true},
+        {name: 'Two', selected: false}
+      ];
+      componentFixture.detectChanges();
+
+      expect(list.options.map(option => option.selected)).toEqual([true, false]);
+
+      // Assign identical data again, but use different
+      // object references so the list is re-rendered.
+      componentFixture.componentInstance.items = [
+        {name: 'One', selected: true},
+        {name: 'Two', selected: false}
+      ];
+      componentFixture.detectChanges();
+
+      expect(list.options.map(option => option.selected)).toEqual([true, false]);
+    });
+
   });
 
   describe('with list option selected', () => {
@@ -1469,3 +1498,19 @@ class SelectionListWithIndirectChildOptions {
 })
 class SelectionListWithIndirectDescendantLines {
 }
+
+@Component({
+  template: `
+    <mat-selection-list>
+      <mat-list-option *ngFor="let item of items" [selected]="item.selected">
+        {{item.name}}
+      </mat-list-option>
+    </mat-selection-list>
+  `
+})
+class SelectionListWithDynamicOptionsWithoutValue {
+  items = [
+    {name: 'One', selected: false},
+    {name: 'Two', selected: false}
+  ];
+}

src/material/list/selection-list.ts

@@ -579,7 +579,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
 
   /** Returns the values of the selected options. */
   private _getSelectedOptionValues(): string[] {
-    return this.options.filter(option => option.selected).map(option => option.value);
+    return this.options.filter(option => {
+      return option.selected && option.value !== undefined;
+    }).map(option => option.value);
   }
 
   /** Toggles the state of the currently focused option if enabled. */