fix(cdk/listbox): remove incorrect usage of validator (#25856)

Currently the CDK listbox is providing itself as a validator so that it can validate that programmatically-assigned values are valid. This is incorrect, because validators are meant to check user-assigned values and the checks that are currently being performed can't happen purely through the UI.

These changes remove the `Validator` implementation and replace the checks with runtime errors.

Author: crisbeto

src/cdk/listbox/listbox.spec.ts

@@ -852,44 +852,28 @@ describe('CdkOption and CdkListbox', () => {
       subscription.unsubscribe();
     });
 
-    it('should have FormControl error when multiple values selected in single-select listbox', () => {
+    it('should throw when multiple values selected in single-select listbox', () => {
       const {testComponent, fixture} = setupComponent(ListboxWithFormControl, [
         ReactiveFormsModule,
       ]);
-      testComponent.formControl.setValue(['orange', 'banana']);
-      fixture.detectChanges();
 
-      expect(testComponent.formControl.hasError('cdkListboxUnexpectedMultipleValues')).toBeTrue();
-      expect(testComponent.formControl.hasError('cdkListboxUnexpectedOptionValues')).toBeFalse();
+      expect(() => {
+        testComponent.formControl.setValue(['orange', 'banana']);
+        fixture.detectChanges();
+      }).toThrowError('Listbox cannot have more than one selected value in multi-selection mode.');
     });
 
-    it('should have FormControl error when non-option value selected', () => {
+    it('should throw when an invalid value is selected', () => {
       const {testComponent, fixture} = setupComponent(ListboxWithFormControl, [
         ReactiveFormsModule,
       ]);
       testComponent.isMultiselectable = true;
-      testComponent.formControl.setValue(['orange', 'dragonfruit', 'mango']);
       fixture.detectChanges();
 
-      expect(testComponent.formControl.hasError('cdkListboxUnexpectedOptionValues')).toBeTrue();
-      expect(testComponent.formControl.hasError('cdkListboxUnexpectedMultipleValues')).toBeFalse();
-      expect(testComponent.formControl.errors?.['cdkListboxUnexpectedOptionValues']).toEqual({
-        'values': ['dragonfruit', 'mango'],
-      });
-    });
-
-    it('should have multiple FormControl errors when multiple non-option values selected in single-select listbox', () => {
-      const {testComponent, fixture} = setupComponent(ListboxWithFormControl, [
-        ReactiveFormsModule,
-      ]);
-      testComponent.formControl.setValue(['dragonfruit', 'mango']);
-      fixture.detectChanges();
-
-      expect(testComponent.formControl.hasError('cdkListboxUnexpectedOptionValues')).toBeTrue();
-      expect(testComponent.formControl.hasError('cdkListboxUnexpectedMultipleValues')).toBeTrue();
-      expect(testComponent.formControl.errors?.['cdkListboxUnexpectedOptionValues']).toEqual({
-        'values': ['dragonfruit', 'mango'],
-      });
+      expect(() => {
+        testComponent.formControl.setValue(['orange', 'dragonfruit', 'mango']);
+        fixture.detectChanges();
+      }).toThrowError('Listbox has selected values that do not match any of its options.');
     });
   });
 });

src/cdk/listbox/listbox.ts

@@ -36,16 +36,7 @@ import {BooleanInput, coerceArray, coerceBooleanProperty} from '@angular/cdk/coe
 import {SelectionModel} from '@angular/cdk/collections';
 import {defer, merge, Observable, Subject} from 'rxjs';
 import {filter, map, startWith, switchMap, takeUntil} from 'rxjs/operators';
-import {
-  AbstractControl,
-  ControlValueAccessor,
-  NG_VALIDATORS,
-  NG_VALUE_ACCESSOR,
-  ValidationErrors,
-  Validator,
-  ValidatorFn,
-  Validators,
-} from '@angular/forms';
+import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
 import {Directionality} from '@angular/cdk/bidi';
 
 /** The next id to use for creating unique DOM IDs. */
@@ -256,16 +247,9 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab
       useExisting: forwardRef(() => CdkListbox),
       multi: true,
     },
-    {
-      provide: NG_VALIDATORS,
-      useExisting: forwardRef(() => CdkListbox),
-      multi: true,
-    },
   ],
 })
-export class CdkListbox<T = unknown>
-  implements AfterContentInit, OnDestroy, ControlValueAccessor, Validator
-{
+export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, ControlValueAccessor {
   /** The id of the option's host element. */
   @Input()
   get id() {
@@ -416,9 +400,6 @@ export class CdkListbox<T = unknown>
   /** Callback called when the listbox value changes */
   private _onChange: (value: readonly T[]) => void = () => {};
 
-  /** Callback called when the form validator changes. */
-  private _onValidatorChange = () => {};
-
   /** Emits when an option has been clicked. */
   private _optionClicked = defer(() =>
     (this.options.changes as Observable<CdkOption<T>[]>).pipe(
@@ -438,40 +419,6 @@ export class CdkListbox<T = unknown>
   /** A predicate that does not skip any options. */
   private readonly _skipNonePredicate = () => false;
 
-  /**
-   * Validator that produces an error if multiple values are selected in a single selection
-   * listbox.
-   * @param control The control to validate
-   * @return A validation error or null
-   */
-  private _validateUnexpectedMultipleValues: ValidatorFn = (control: AbstractControl) => {
-    const controlValue = this._coerceValue(control.value);
-    if (!this.multiple && controlValue.length > 1) {
-      return {'cdkListboxUnexpectedMultipleValues': true};
-    }
-    return null;
-  };
-
-  /**
-   * Validator that produces an error if any selected values are not valid options for this listbox.
-   * @param control The control to validate
-   * @return A validation error or null
-   */
-  private _validateUnexpectedOptionValues: ValidatorFn = (control: AbstractControl) => {
-    const controlValue = this._coerceValue(control.value);
-    const invalidValues = this._getInvalidOptionValues(controlValue);
-    if (invalidValues.length) {
-      return {'cdkListboxUnexpectedOptionValues': {'values': invalidValues}};
-    }
-    return null;
-  };
-
-  /** The combined set of validators for this listbox. */
-  private _validators = Validators.compose([
-    this._validateUnexpectedMultipleValues,
-    this._validateUnexpectedOptionValues,
-  ])!;
-
   ngAfterContentInit() {
     if (typeof ngDevMode === 'undefined' || ngDevMode) {
       this._verifyNoOptionValueCollisions();
@@ -614,6 +561,19 @@ export class CdkListbox<T = unknown>
    */
   writeValue(value: readonly T[]): void {
     this._setSelection(value);
+
+    if (typeof ngDevMode === 'undefined' || ngDevMode) {
+      const selected = this.selectionModel.selected;
+      const invalidValues = this._getInvalidOptionValues(selected);
+
+      if (!this.multiple && selected.length > 1) {
+        throw Error('Listbox cannot have more than one selected value in multi-selection mode.');
+      }
+
+      if (invalidValues.length) {
+        throw Error('Listbox has selected values that do not match any of its options.');
+      }
+    }
   }
 
   /**
@@ -625,23 +585,6 @@ export class CdkListbox<T = unknown>
     this.disabled = isDisabled;
   }
 
-  /**
-   * Validate the given control
-   * @docs-private
-   */
-  validate(control: AbstractControl<any, any>): ValidationErrors | null {
-    return this._validators(control);
-  }
-
-  /**
-   * Registers a callback to be called when the form validator changes.
-   * @param fn The callback to call
-   * @docs-private
-   */
-  registerOnValidatorChange(fn: () => void) {
-    this._onValidatorChange = fn;
-  }
-
   /** Focus the listbox's host element. */
   focus() {
     this.element.focus();
@@ -901,7 +844,6 @@ export class CdkListbox<T = unknown>
     const selected = this.selectionModel.selected;
     this._invalid =
       (!this.multiple && selected.length > 1) || !!this._getInvalidOptionValues(selected).length;
-    this._onValidatorChange();
     this.changeDetectorRef.markForCheck();
   }
 

tools/public_api_guard/cdk/listbox.md

@@ -4,7 +4,6 @@
 
 ```ts
 
-import { AbstractControl } from '@angular/forms';
 import { ActiveDescendantKeyManager } from '@angular/cdk/a11y';
 import { AfterContentInit } from '@angular/core';
 import { BooleanInput } from '@angular/cdk/coercion';
@@ -17,11 +16,9 @@ import { OnDestroy } from '@angular/core';
 import { QueryList } from '@angular/core';
 import { SelectionModel } from '@angular/cdk/collections';
 import { Subject } from 'rxjs';
-import { ValidationErrors } from '@angular/forms';
-import { Validator } from '@angular/forms';
 
 // @public (undocumented)
-export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, ControlValueAccessor, Validator {
+export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, ControlValueAccessor {
     protected readonly changeDetectorRef: ChangeDetectorRef;
     get compareWith(): undefined | ((o1: T, o2: T) => boolean);
     set compareWith(fn: undefined | ((o1: T, o2: T) => boolean));
@@ -59,7 +56,6 @@ export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, Con
     set orientation(value: 'horizontal' | 'vertical');
     registerOnChange(fn: (value: readonly T[]) => void): void;
     registerOnTouched(fn: () => {}): void;
-    registerOnValidatorChange(fn: () => void): void;
     select(option: CdkOption<T>): void;
     protected selectionModel: ListboxSelectionModel<T>;
     selectValue(value: T): void;
@@ -72,7 +68,6 @@ export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, Con
     protected triggerRange(trigger: CdkOption<T> | null, from: number, to: number, on: boolean): void;
     get useActiveDescendant(): boolean;
     set useActiveDescendant(shouldUseActiveDescendant: BooleanInput);
-    validate(control: AbstractControl<any, any>): ValidationErrors | null;
     get value(): readonly T[];
     set value(value: readonly T[]);
     readonly valueChange: Subject<ListboxValueChangeEvent<T>>;