fix(select): unable to use the MatOption select/deselect API to toggle options

src/cdk/collections/selection.spec.ts

@@ -82,6 +82,14 @@ describe('SelectionModel', () => {
 
       expect(model.selected).toEqual([1, 2, 3]);
     });
+
+    it('should sort values if `selected` has not been accessed before', () => {
+      model = new SelectionModel(true, [2, 3, 1]);
+
+      // Important: don't assert `selected` before sorting so the getter isn't invoked
+      model.sort();
+      expect(model.selected).toEqual([1, 2, 3]);
+    });
   });
 
   describe('onChange event', () => {

src/cdk/collections/selection.ts

@@ -13,7 +13,7 @@ import {Subject} from 'rxjs';
  */
 export class SelectionModel<T> {
   /** Currently-selected values. */
-  private _selection: Set<T> = new Set();
+  private _selection = new Set<T>();
 
   /** Keeps track of the deselected options that haven't been emitted by the change event. */
   private _deselectedToEmit: T[] = [];
@@ -111,8 +111,8 @@ export class SelectionModel<T> {
    * Sorts the selected values based on a predicate function.
    */
   sort(predicate?: (a: T, b: T) => number): void {
-    if (this._multiple && this._selected) {
-      this._selected.sort(predicate);
+    if (this._multiple && this.selected) {
+      this._selected!.sort(predicate);
     }
   }
 

src/lib/core/option/option.spec.ts

@@ -27,6 +27,50 @@ describe('MatOption component', () => {
     subscription.unsubscribe();
   });
 
+  it('should not emit to `onSelectionChange` if selecting an already-selected option', () => {
+    const fixture = TestBed.createComponent(OptionWithDisable);
+    fixture.detectChanges();
+
+    const optionInstance: MatOption =
+        fixture.debugElement.query(By.directive(MatOption)).componentInstance;
+
+    optionInstance.select();
+    expect(optionInstance.selected).toBe(true);
+
+    const spy = jasmine.createSpy('selection change spy');
+    const subscription = optionInstance.onSelectionChange.subscribe(spy);
+
+    optionInstance.select();
+    fixture.detectChanges();
+
+    expect(optionInstance.selected).toBe(true);
+    expect(spy).not.toHaveBeenCalled();
+
+    subscription.unsubscribe();
+  });
+
+  it('should not emit to `onSelectionChange` if deselecting an unselected option', () => {
+    const fixture = TestBed.createComponent(OptionWithDisable);
+    fixture.detectChanges();
+
+    const optionInstance: MatOption =
+        fixture.debugElement.query(By.directive(MatOption)).componentInstance;
+
+    optionInstance.deselect();
+    expect(optionInstance.selected).toBe(false);
+
+    const spy = jasmine.createSpy('selection change spy');
+    const subscription = optionInstance.onSelectionChange.subscribe(spy);
+
+    optionInstance.deselect();
+    fixture.detectChanges();
+
+    expect(optionInstance.selected).toBe(false);
+    expect(spy).not.toHaveBeenCalled();
+
+    subscription.unsubscribe();
+  });
+
   describe('ripples', () => {
     let fixture: ComponentFixture<OptionWithDisable>;
     let optionDebugElement: DebugElement;

src/lib/core/option/option.ts

@@ -145,16 +145,20 @@ export class MatOption implements AfterViewChecked, OnDestroy {
 
   /** Selects the option. */
   select(): void {
-    this._selected = true;
-    this._changeDetectorRef.markForCheck();
-    this._emitSelectionChangeEvent();
+    if (!this._selected) {
+      this._selected = true;
+      this._changeDetectorRef.markForCheck();
+      this._emitSelectionChangeEvent();
+    }
   }
 
   /** Deselects the option. */
   deselect(): void {
-    this._selected = false;
-    this._changeDetectorRef.markForCheck();
-    this._emitSelectionChangeEvent();
+    if (this._selected) {
+      this._selected = false;
+      this._changeDetectorRef.markForCheck();
+      this._emitSelectionChangeEvent();
+    }
   }
 
   /** Sets focus onto this option. */

src/lib/select/select.spec.ts

@@ -86,7 +86,7 @@ describe('MatSelect', () => {
    * overall test time.
    * @param declarations Components to declare for this block
    */
-  function configureMatSelectTestingModule(declarations) {
+  function configureMatSelectTestingModule(declarations: any[]) {
     TestBed.configureTestingModule({
       imports: [
         MatFormFieldModule,
@@ -1096,6 +1096,23 @@ describe('MatSelect', () => {
             .toBe(fixture.componentInstance.options.first);
       }));
 
+      it('should be able to select an option using the MatOption API', fakeAsync(() => {
+        trigger.click();
+        fixture.detectChanges();
+        flush();
+
+        const optionInstances = fixture.componentInstance.options.toArray();
+        const optionNodes: NodeListOf<HTMLElement> =
+            overlayContainerElement.querySelectorAll('mat-option');
+
+        optionInstances[1].select();
+        fixture.detectChanges();
+
+        expect(optionNodes[1].classList).toContain('mat-selected');
+        expect(optionInstances[1].selected).toBe(true);
+        expect(fixture.componentInstance.select.selected).toBe(optionInstances[1]);
+      }));
+
       it('should deselect other options when one is selected', fakeAsync(() => {
         trigger.click();
         fixture.detectChanges();

src/lib/select/select.ts

@@ -468,13 +468,18 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
   }
 
   ngOnInit() {
-    this._selectionModel = new SelectionModel<MatOption>(this.multiple, undefined, false);
+    this._selectionModel = new SelectionModel<MatOption>(this.multiple);
     this.stateChanges.next();
   }
 
   ngAfterContentInit() {
     this._initKeyManager();
 
+    this._selectionModel.onChange!.pipe(takeUntil(this._destroy)).subscribe(event => {
+      event.added.forEach(option => option.select());
+      event.removed.forEach(option => option.deselect());
+    });
+
     this.options.changes.pipe(startWith(null), takeUntil(this._destroy)).subscribe(() => {
       this._resetOptions();
       this._initializeSelection();
@@ -753,19 +758,18 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
    * Sets the selected option based on a value. If no option can be
    * found with the designated value, the select trigger is cleared.
    */
-  private _setSelectionByValue(value: any | any[], isUserInput = false): void {
+  private _setSelectionByValue(value: any | any[]): void {
     if (this.multiple && value) {
       if (!Array.isArray(value)) {
         throw getMatSelectNonArrayValueError();
       }
 
-      this._clearSelection();
-      value.forEach((currentValue: any) => this._selectValue(currentValue, isUserInput));
+      this._selectionModel.clear();
+      value.forEach((currentValue: any) => this._selectValue(currentValue));
       this._sortValues();
     } else {
-      this._clearSelection();
-
-      const correspondingOption = this._selectValue(value, isUserInput);
+      this._selectionModel.clear();
+      const correspondingOption = this._selectValue(value);
 
       // Shift focus to the active item. Note that we shouldn't do this in multiple
       // mode, because we don't know what option the user interacted with last.
@@ -781,7 +785,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
    * Finds and selects and option based on its value.
    * @returns Option that has the corresponding value.
    */
-  private _selectValue(value: any, isUserInput = false): MatOption | undefined {
+  private _selectValue(value: any): MatOption | undefined {
     const correspondingOption = this.options.find((option: MatOption) => {
       try {
         // Treat null as a special reset value.
@@ -796,29 +800,12 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
     });
 
     if (correspondingOption) {
-      isUserInput ? correspondingOption._selectViaInteraction() : correspondingOption.select();
       this._selectionModel.select(correspondingOption);
-      this.stateChanges.next();
     }
 
     return correspondingOption;
   }
 
-
-  /**
-   * Clears the select trigger and deselects every option in the list.
-   * @param skip Option that should not be deselected.
-   */
-  private _clearSelection(skip?: MatOption): void {
-    this._selectionModel.clear();
-    this.options.forEach(option => {
-      if (option !== skip) {
-        option.deselect();
-      }
-    });
-    this.stateChanges.next();
-  }
-
   /** Sets up a key manager to listen to keyboard events on the overlay panel. */
   private _initKeyManager() {
     this._keyManager = new ActiveDescendantKeyManager<MatOption>(this.options)
@@ -846,16 +833,14 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
   private _resetOptions(): void {
     const changedOrDestroyed = merge(this.options.changes, this._destroy);
 
-    this.optionSelectionChanges
-      .pipe(takeUntil(changedOrDestroyed), filter(event => event.isUserInput))
-      .subscribe(event => {
-        this._onSelect(event.source);
+    this.optionSelectionChanges.pipe(takeUntil(changedOrDestroyed)).subscribe(event => {
+      this._onSelect(event.source, event.isUserInput);
 
-        if (!this.multiple && this._panelOpen) {
-          this.close();
-          this.focus();
-        }
-      });
+      if (event.isUserInput && !this.multiple && this._panelOpen) {
+        this.close();
+        this.focus();
+      }
+    });
 
     // Listen to changes in the internal state of the options and react accordingly.
     // Handles cases like the labels of the selected options changing.
@@ -870,51 +855,42 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
   }
 
   /** Invoked when an option is clicked. */
-  private _onSelect(option: MatOption): void {
+  private _onSelect(option: MatOption, isUserInput: boolean): void {
     const wasSelected = this._selectionModel.isSelected(option);
 
-    // TODO(crisbeto): handle blank/null options inside multi-select.
-    if (this.multiple) {
-      this._selectionModel.toggle(option);
-      this.stateChanges.next();
-      wasSelected ? option.deselect() : option.select();
-      this._keyManager.setActiveItem(option);
-      this._sortValues();
-
-      // In case the user select the option with their mouse, we
-      // want to restore focus back to the trigger, in order to
-      // prevent the select keyboard controls from clashing with
-      // the ones from `mat-option`.
-      this.focus();
+    if (option.value == null) {
+      this._selectionModel.clear();
+      this._propagateChanges(option.value);
     } else {
-      this._clearSelection(option.value == null ? undefined : option);
-
-      if (option.value == null) {
-        this._propagateChanges(option.value);
-      } else {
-        this._selectionModel.select(option);
-        this.stateChanges.next();
+      option.selected ? this._selectionModel.select(option) : this._selectionModel.deselect(option);
+
+      // TODO(crisbeto): handle blank/null options inside multi-select.
+      if (this.multiple) {
+        this._sortValues();
+
+        if (isUserInput) {
+          this._keyManager.setActiveItem(option);
+          // In case the user selected the option with their mouse, we
+          // want to restore focus back to the trigger, in order to
+          // prevent the select keyboard controls from clashing with
+          // the ones from `mat-option`.
+          this.focus();
+        }
       }
     }
 
     if (wasSelected !== this._selectionModel.isSelected(option)) {
       this._propagateChanges();
     }
-  }
 
-  /**
-   * Sorts the model values, ensuring that they keep the same
-   * order that they have in the panel.
-   */
-  private _sortValues(): void {
-    if (this._multiple) {
-      this._selectionModel.clear();
+    this.stateChanges.next();
+  }
 
-      this.options.forEach(option => {
-        if (option.selected) {
-          this._selectionModel.select(option);
-        }
-      });
+  /** Sorts the selected values in the selected based on their order in the panel. */
+  private _sortValues() {
+    if (this.multiple) {
+      const options = this.options.toArray();
+      this._selectionModel.sort((a, b) => options.indexOf(a) - options.indexOf(b));
       this.stateChanges.next();
     }
   }