From e441ff44b431759a7a020c18dcde70e14dd29a59 Mon Sep 17 00:00:00 2001
From: Dylan Hunn <dylhunn@gmail.com>
Date: Tue, 10 May 2022 00:14:10 -0700
Subject: [PATCH] fix(forms): Prevent FormBuilder from distributing unions to
 control types. (#45942)

Previously, using `FormBuilder` with a union type would produce unions of *controls*:

```
// `foo` has type `FormControl<string>|FormControl<number>`.
const c = fb.nonNullable.group({foo: 'bar' as string | number});
```

This actually works in many cases, due to how extraordinarily powerful Typescript's distributive types are (e.g. `value` still has type `string|number`), but it is subtly incorrect. Here is a code example that exposes the reason the inference is incorrect. It exploits the fact that Typescript will not "un-distribute" a type, producing an obviously spurious error:

```
// fc gets an inferred distributive union type `FormControl<string> | FormControl<number>`
let fc = c.controls.foo;
// Error: Type 'FormControl<string | number>' is not assignable to type 'FormControl<string> | FormControl<number>'.
fc = new FormControl<string|number>('', {initialValueIsDefault: true});
```

Instead, we want the union to apply to the *values*:

```
// `foo` should have type `FormControl<string|number>`.
const c = fb.nonNullable.group({foo: 'bar' as string | number});
```

Essentially, we want to prevent Typescript from distributing the type. [As specified in the handbook](https://www.typescriptlang.org/docs/handbook/2/conditional-types.html#distributive-conditional-types):

> Typically, distributivity is the desired behavior. To avoid that behavior, you can surround each side of the extends keyword with square brackets.

This PR applies this suggestion to `FormBuilder`'s type inference.

Fixes #45912.

PR Close #45942
---
 packages/forms/src/form_builder.ts            | 17 ++++++++++-------
 packages/forms/test/typed_integration_spec.ts | 12 ++++++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/packages/forms/src/form_builder.ts b/packages/forms/src/form_builder.ts
index 306b3e925a6..86c7e1baa20 100644
--- a/packages/forms/src/form_builder.ts
+++ b/packages/forms/src/form_builder.ts
@@ -48,16 +48,19 @@ export type ControlConfig<T> = [T|FormControlState<T>, (ValidatorFn|(ValidatorFn
  * The flag N, if not never, makes the resulting `FormControl` have N in its type. 
  */
 export type ɵElement<T, N extends null> =
-  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|N> :
-  T extends ControlConfig<infer U> ? FormControl<U|N> :
+  // The `extends` checks are wrapped in arrays in order to prevent TypeScript from applying type unions
+  // through the distributive conditional type. This is the officially recommended solution:
+  // https://www.typescriptlang.org/docs/handbook/2/conditional-types.html#distributive-conditional-types
+  [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|N> :
+  [T] extends [ControlConfig<infer U>] ? FormControl<U|N> :
   // 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|N> :
+  [T] extends [Array<infer U|ValidatorFn|ValidatorFn[]|AsyncValidatorFn|AsyncValidatorFn[]>] ? FormControl<U|N> :
   // Fallthough case: T is not a container type; use it directly as a value.
   FormControl<T|N>;
 
diff --git a/packages/forms/test/typed_integration_spec.ts b/packages/forms/test/typed_integration_spec.ts
index 3fd881da828..0092e7ba9e2 100644
--- a/packages/forms/test/typed_integration_spec.ts
+++ b/packages/forms/test/typed_integration_spec.ts
@@ -1366,6 +1366,18 @@ describe('Typed Class', () => {
         expect(c.value).toEqual({foo: 'bar'});
       });
 
+      it('without distributing union types', () => {
+        const c = fb.group({foo: 'bar' as string | number});
+        {
+          type ControlsType = {foo: FormControl<string|number>};
+          let t: ControlsType = c.controls;
+          let t1 = c.controls;
+          t1 = null as unknown as ControlsType;
+        }
+        let fc = c.controls.foo;
+        fc = new FormControl<string|number>('', {initialValueIsDefault: true});
+      });
+
       describe('from objects with FormControls', () => {
         it('from objects with builder FormGroups', () => {
           const c = fb.group({foo: fb.group({baz: 'bar'})});