fix(forms): Fix a typing bug in FormBuilder. (#45684)

dylhunn · dylhunn · commit ff3f5a8d12e3 · 2022-04-20T09:15:46.000-07:00
Previously, the following code would fail to compile: ``` let form: FormGroup<{email: FormControl<string | null>}>; form = fb.group({ email: ['', Validators.required] }); ``` This is because the compiler was unable to properly infer the inner type of `ControlConfig` arrays in some cases. The same issue applies to `FormArray` as well under certain circumstances. This change cleans up the `FormBuilder` type signatures to always use the explicit Element type, and to catch `ControlConfig` types that might fall through. PR Close #45684
diff --git a/goldens/public-api/forms/index.md b/goldens/public-api/forms/index.md
@@ -276,29 +276,16 @@ export class FormArrayName extends ControlContainer implements OnInit, OnDestroy
 
 // @public
 export class FormBuilder {
-    // (undocumented)
-    array<T>(controls: Array<FormControl<T>>, validatorOrOpts?: ValidatorFn | ValidatorFn[] | AbstractControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormArray<FormControl<T>>;
-    // (undocumented)
-    array<T extends {
-        [K in keyof T]: AbstractControl<any>;
-    }>(controls: Array<FormGroup<T>>, validatorOrOpts?: ValidatorFn | ValidatorFn[] | AbstractControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormArray<FormGroup<T>>;
-    // (undocumented)
-    array<T extends AbstractControl<any>>(controls: Array<FormArray<T>>, validatorOrOpts?: ValidatorFn | ValidatorFn[] | AbstractControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormArray<FormArray<T>>;
-    // (undocumented)
-    array<T extends AbstractControl<any>>(controls: Array<AbstractControl<T>>, validatorOrOpts?: ValidatorFn | ValidatorFn[] | AbstractControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormArray<AbstractControl<T>>;
-    // (undocumented)
-    array<T>(controls: Array<FormControlState<T> | ControlConfig<T> | T>, validatorOrOpts?: ValidatorFn | ValidatorFn[] | AbstractControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormArray<FormControl<T | null>>;
+    array<T>(controls: Array<T>, validatorOrOpts?: ValidatorFn | ValidatorFn[] | AbstractControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormArray<ɵElement<T>>;
     // (undocumented)
     control<T>(formState: T | FormControlState<T>, opts: FormControlOptions & {
         initialValueIsDefault: true;
     }): FormControl<T>;
     // (undocumented)
     control<T>(formState: T | FormControlState<T>, validatorOrOpts?: ValidatorFn | ValidatorFn[] | FormControlOptions | null, asyncValidator?: AsyncValidatorFn | AsyncValidatorFn[] | null): FormControl<T | null>;
     // (undocumented)
-    group<T extends {
-        [K in keyof T]: FormControlState<any> | ControlConfig<any> | FormControl<any> | FormGroup<any> | FormArray<any> | AbstractControl<any> | T[K];
-    }>(controls: T, options?: AbstractControlOptions | null): FormGroup<{
-        [K in keyof T]: ɵGroupElement<T[K]>;
+    group<T extends {}>(controls: T, options?: AbstractControlOptions | null): FormGroup<{
+        [K in keyof T]: ɵElement<T[K]>;
     }>;
     // @deprecated
     group(controls: {
diff --git a/packages/forms/src/form_builder.ts b/packages/forms/src/form_builder.ts
@@ -15,11 +15,19 @@ import {FormArray, UntypedFormArray} from './model/form_array';
 import {FormControl, FormControlOptions, FormControlState, UntypedFormControl} from './model/form_control';
 import {FormGroup, UntypedFormGroup} from './model/form_group';
 
-function isAbstractControlOptions(options: AbstractControlOptions|
-                                  {[key: string]: any}): options is AbstractControlOptions {
-  return (<AbstractControlOptions>options).asyncValidators !== undefined ||
-      (<AbstractControlOptions>options).validators !== undefined ||
-      (<AbstractControlOptions>options).updateOn !== undefined;
+function isAbstractControlOptions(options: AbstractControlOptions|{[key: string]: any}|null|
+                                  undefined): options is AbstractControlOptions {
+  return !!options &&
+      ((options as AbstractControlOptions).asyncValidators !== undefined ||
+       (options as AbstractControlOptions).validators !== undefined ||
+       (options as AbstractControlOptions).updateOn !== undefined);
+}
+
+function isFormControlOptions(options: FormControlOptions|{[key: string]: any}|null|
+                              undefined): options is FormControlOptions {
+  return !!options &&
+      (isAbstractControlOptions(options) ||
+       (options as FormControlOptions).initialValueIsDefault !== undefined);
 }
 
 /**
@@ -30,15 +38,25 @@ function isAbstractControlOptions(options: AbstractControlOptions|
  */
 export type ControlConfig<T> = [T|FormControlState<T>, (ValidatorFn|(ValidatorFn[]))?, (AsyncValidatorFn|AsyncValidatorFn[])?];
 
-// Disable clang-format to produce clearer formatting for this multiline type.
+// Disable clang-format to produce clearer formatting for these multiline types.
 // clang-format off
-export type ɵGroupElement<T> =
+
+/**
+ * FormBuilder accepts values in various container shapes, as well as raw values.
+ * Element returns the appropriate corresponding model class.
+ */
+export type ɵElement<T> =
   T extends FormControl<infer U> ? FormControl<U> :
   T extends FormGroup<infer U> ? FormGroup<U> :
   T extends FormArray<infer U> ? FormArray<U> :
   T extends AbstractControl<infer U> ? AbstractControl<U> :
   T extends FormControlState<infer U> ? FormControl<U|null> :
   T extends ControlConfig<infer U> ? FormControl<U|null> :
+  // ControlConfig can be too much for the compiler to infer in the wrapped case. This is
+  // not surprising, since it's practically death-by-polymorphism (e.g. the optional validators
+  // members that might be arrays). Watch for ControlConfigs that might fall through.
+  T extends Array<infer U|ValidatorFn|ValidatorFn[]|AsyncValidatorFn|AsyncValidatorFn[]> ? FormControl<U|null> :
+  // Fallthough case: T is not a container type; use is directly as a value.
   FormControl<T|null>;
 
 // clang-format on
@@ -57,13 +75,10 @@ export type ɵGroupElement<T> =
  */
 @Injectable({providedIn: ReactiveFormsModule})
 export class FormBuilder {
-  group<T extends {
-    [K in keyof T]: FormControlState<any>| ControlConfig<any>| FormControl<any>| FormGroup<any>|
-        FormArray<any>| AbstractControl<any>| T[K]
-  }>(
+  group<T extends {}>(
       controls: T,
       options?: AbstractControlOptions|null,
-      ): FormGroup<{[K in keyof T]: ɵGroupElement<T[K]>}>;
+      ): FormGroup<{[K in keyof T]: ɵElement<T[K]>}>;
 
   /**
    * @description
@@ -124,12 +139,14 @@ export class FormBuilder {
         updateOn = options.updateOn != null ? options.updateOn : undefined;
       } else {
         // `options` are legacy form group options
-        validators = options['validator'] != null ? options['validator'] : null;
-        asyncValidators = options['asyncValidator'] != null ? options['asyncValidator'] : null;
+        validators = (options as any)['validator'] != null ? (options as any)['validator'] : null;
+        asyncValidators =
+            (options as any)['asyncValidator'] != null ? (options as any)['asyncValidator'] : null;
       }
     }
 
-    return new FormGroup(reducedControls, {asyncValidators, updateOn, validators});
+    // Cast to `any` because the inferred types are not as specific as Element.
+    return new FormGroup(reducedControls, {asyncValidators, updateOn, validators}) as any;
   }
 
   control<T>(formState: T|FormControlState<T>, opts: FormControlOptions&{
@@ -169,36 +186,10 @@ export class FormBuilder {
   control<T>(
       formState: T|FormControlState<T>,
       validatorOrOpts?: ValidatorFn|ValidatorFn[]|FormControlOptions|null,
-      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|
-      null): FormControl<T|null>|FormControl<T> {
+      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormControl {
     return new FormControl(formState, validatorOrOpts, asyncValidator);
   }
 
-  array<T>(
-      controls: Array<FormControl<T>>,
-      validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
-      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormArray<FormControl<T>>;
-
-  array<T extends {[K in keyof T]: AbstractControl<any>}>(
-      controls: Array<FormGroup<T>>,
-      validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
-      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormArray<FormGroup<T>>;
-
-  array<T extends AbstractControl<any>>(
-      controls: Array<FormArray<T>>,
-      validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
-      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormArray<FormArray<T>>;
-
-  array<T extends AbstractControl<any>>(
-      controls: Array<AbstractControl<T>>,
-      validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
-      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormArray<AbstractControl<T>>;
-
-  array<T>(
-      controls: Array<FormControlState<T>|ControlConfig<T>|T>,
-      validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
-      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormArray<FormControl<T|null>>;
-
   /**
    * Constructs a new `FormArray` from the given array of configurations,
    * validators and options.
@@ -212,11 +203,12 @@ export class FormBuilder {
    *
    * @param asyncValidator A single async validator or array of async validator functions.
    */
-  array(
-      controls: any[], validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
-      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormArray {
+  array<T>(
+      controls: Array<T>, validatorOrOpts?: ValidatorFn|ValidatorFn[]|AbstractControlOptions|null,
+      asyncValidator?: AsyncValidatorFn|AsyncValidatorFn[]|null): FormArray<ɵElement<T>> {
     const createdControls = controls.map(c => this._createControl(c));
-    return new FormArray(createdControls, validatorOrOpts, asyncValidator);
+    // Cast to `any` because the inferred types are not as specific as Element.
+    return new FormArray(createdControls, validatorOrOpts, asyncValidator) as any;
   }
 
   /** @internal */
@@ -261,13 +253,16 @@ export class UntypedFormBuilder extends FormBuilder {
       controlsConfig: {[key: string]: any},
       options?: AbstractControlOptions|null,
       ): UntypedFormGroup;
+
   /**
-   * @deprecated
+   * @deprecated This API is not typesafe and can result in issues with Closure Compiler renaming.
+   * Use the `FormBuilder#group` overload with `AbstractControlOptions` instead.
    */
   override group(
       controlsConfig: {[key: string]: any},
       options: {[key: string]: any},
       ): UntypedFormGroup;
+
   override group(
       controlsConfig: {[key: string]: any},
       options: AbstractControlOptions|{[key: string]: any}|null = null): UntypedFormGroup {
diff --git a/packages/forms/src/forms.ts b/packages/forms/src/forms.ts
@@ -41,7 +41,7 @@ export {FormArrayName, FormGroupName} from './directives/reactive_directives/for
 export {NgSelectOption, SelectControlValueAccessor} from './directives/select_control_value_accessor';
 export {SelectMultipleControlValueAccessor, ɵNgSelectMultipleOption} from './directives/select_multiple_control_value_accessor';
 export {AsyncValidator, AsyncValidatorFn, CheckboxRequiredValidator, EmailValidator, MaxLengthValidator, MaxValidator, MinLengthValidator, MinValidator, PatternValidator, RequiredValidator, ValidationErrors, Validator, ValidatorFn} from './directives/validators';
-export {FormBuilder, UntypedFormBuilder, ɵGroupElement} from './form_builder';
+export {FormBuilder, UntypedFormBuilder, ɵElement} from './form_builder';
 export {AbstractControl, AbstractControlOptions, FormControlStatus, ɵCoerceStrArrToNumArr, ɵGetProperty, ɵNavigate, ɵRawValue, ɵTokenize, ɵTypedOrUntyped, ɵValue, ɵWriteable} from './model/abstract_model';
 export {FormArray, UntypedFormArray, ɵFormArrayRawValue, ɵFormArrayValue} from './model/form_array';
 export {FormControl, FormControlOptions, FormControlState, UntypedFormControl, ɵFormControlCtor} from './model/form_control';
diff --git a/packages/forms/test/typed_integration_spec.ts b/packages/forms/test/typed_integration_spec.ts
@@ -1052,6 +1052,37 @@ describe('Typed Class', () => {
         }
       });
 
+      it('from objects with ControlConfigs and validators', () => {
+        const c = fb.group({foo: ['bar', Validators.required]});
+        {
+          type ControlsType = {foo: FormControl<string|null>};
+          let t: ControlsType = c.controls;
+          let t1 = c.controls;
+          t1 = null as unknown as ControlsType;
+        }
+      });
+
+      it('from objects with ControlConfigs and validator lists', () => {
+        const c = fb.group({foo: ['bar', [Validators.required, Validators.email]]});
+        {
+          type ControlsType = {foo: FormControl<string|null>};
+          let t: ControlsType = c.controls;
+          let t1 = c.controls;
+          t1 = null as unknown as ControlsType;
+        }
+      });
+
+      it('from objects with ControlConfigs and explicit types', () => {
+        const c: FormGroup<{foo: FormControl<string|null>}> =
+            fb.group({foo: ['bar', [Validators.required, Validators.email]]});
+        {
+          type ControlsType = {foo: FormControl<string|null>};
+          let t: ControlsType = c.controls;
+          let t1 = c.controls;
+          t1 = null as unknown as ControlsType;
+        }
+      });
+
       describe('from objects with FormControls', () => {
         it('nullably', () => {
           const c = fb.group({foo: new FormControl('bar')});
@@ -1233,6 +1264,7 @@ describe('Typed Class', () => {
           }>>,
         };
         let t: ControlType = myParty.controls;
+        let d = myParty.controls.dinnerOptions;
         let t1 = myParty.controls;
         t1 = null as unknown as ControlType;
       }
