fix(forms): allow FormBuilder.group(...) to accept optional fields. (#46253)

Consider the case in which `FormBuilder` is used to construct a group with an optional field:

```
const controls = { name: fb.control('') };
const foo: FormGroup<{
  name: FormControl<string | null>;
  address?: FormControl<string | null>;
}> = fb.group<{
  name: FormControl<string | null>;
  address?: FormControl<string | null>;
}>(controls);
```

Today, with fully strict TypeScript settings, the above will not compile:

```
Types of property 'controls' are incompatible.
Type '{ name: FormControl<string | null>; address?: FormControl<FormGroup<SubFormControls> | null | undefined> | undefined; }' is not assignable to type '{ name: FormControl<string | null>; address?: FormGroup<SubFormControls> | undefined; }'.
```

Notice that the `fb.group(...)` is calculating the following type for address: `address?: FormControl<FormGroup<string|null>`. This is clearly wrong -- an extraneous `FormControl` has been added!

This is coming from the calculation of the result type of `fb.group(...)`. In the type definition, if we cannot detect the outer control type, [we assume it's just an unwrapped value, and automatically wrap it in `FormControl`](https://github.com/angular/angular/blob/14.0.0/packages/forms/src/form_builder.ts#L66).

Because the optional `{address?: FormControl<string|null>}` implicitly makes the RHS have type `FormControl<string|null>|undefined`, [the relevant condition is not satisfied](https://github.com/angular/angular/blob/14.0.0/packages/forms/src/form_builder.ts#L55). In particular, the condition expects just `FormGroup<T>`, not `FormGroup<T>|undefined`. So we assume `T` is a value type, and it gets wrapped with `FormControl`.

The solution is to add the cases where `undefined` is included in the union type when detecting which control `T` is (if any).

PR Close #46253
This commit is contained in:
Dylan Hunn 2022-06-03 14:50:36 -07:00 committed by Alex Rickabaugh
parent 0d10fe52f1
commit f18e1739b8
4 changed files with 31 additions and 8 deletions

View file

@ -548,9 +548,6 @@
{
"name": "_randomChar"
},
{
"name": "componentDefCount"
},
{
"name": "_symbolIterator"
},
@ -656,6 +653,9 @@
{
"name": "collectStylingFromTAttrs"
},
{
"name": "componentDefCount"
},
{
"name": "compose"
},
@ -1595,4 +1595,4 @@
{
"name": "ɵɵtext"
}
]
]

View file

@ -539,9 +539,6 @@
{
"name": "_randomChar"
},
{
"name": "componentDefCount"
},
{
"name": "_symbolIterator"
},
@ -638,6 +635,9 @@
{
"name": "collectStylingFromTAttrs"
},
{
"name": "componentDefCount"
},
{
"name": "composeAsyncValidators"
},
@ -1583,4 +1583,4 @@
{
"name": "ɵɵtext"
}
]
]

View file

@ -52,11 +52,26 @@ export type ɵElement<T, N extends null> =
// 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
//
// Identify FormControl container types.
[T] extends [FormControl<infer U>] ? FormControl<U> :
// Or FormControl containers that are optional in their parent group.
[T] extends [FormControl<infer U>|undefined] ? FormControl<U> :
// FormGroup containers.
[T] extends [FormGroup<infer U>] ? FormGroup<U> :
// Optional FormGroup containers.
[T] extends [FormGroup<infer U>|undefined] ? FormGroup<U> :
// FormArray containers.
[T] extends [FormArray<infer U>] ? FormArray<U> :
// Optional FormArray containers.
[T] extends [FormArray<infer U>|undefined] ? FormArray<U> :
// Otherwise unknown AbstractControl containers.
[T] extends [AbstractControl<infer U>] ? AbstractControl<U> :
// Optional AbstractControl containers.
[T] extends [AbstractControl<infer U>|undefined] ? AbstractControl<U> :
// FormControlState object container, which produces a nullable control.
[T] extends [FormControlState<infer U>] ? FormControl<U|N> :
// A ControlConfig tuple, which produces a nullable control.
[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

View file

@ -1032,6 +1032,14 @@ describe('Typed Class', () => {
}
});
it('from objects with optional keys', () => {
const controls = {name: fb.control('')};
const foo:
FormGroup<{name: FormControl<string|null>; address?: FormControl<string|null>;}> =
fb.group<{name: FormControl<string|null>; address?: FormControl<string|null>;}>(
controls);
});
it('from objects with FormControlState', () => {
const c = fb.group({foo: {value: 'bar', disabled: false}});
{