Skip to content

Commit

Permalink
fix(forms): Correctly infer FormBuilder types involving `[value, va…
Browse files Browse the repository at this point in the history
…lidators]` shorthand in more cases. (#47034)

Type inference in cases involving `ControlConfig` was previously not working as desired. This was because the compiler was enforcing that `ControlConfig` is a *tuple* -- which is not always that easy to prove! By relaxing this constraint a bit, and just inferring from `ControlConfig` as an array, the type inference catches many more cases, and is generally more correct.

PR Close #47034
  • Loading branch information
dylhunn authored and Pawel Kozlowski committed Aug 17, 2022
1 parent dc52cef commit b302797
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 7 deletions.
23 changes: 16 additions & 7 deletions packages/forms/src/form_builder.ts
Expand Up @@ -23,6 +23,21 @@ function isAbstractControlOptions(options: AbstractControlOptions|{[key: string]
(options as AbstractControlOptions).updateOn !== undefined);
}

/**
* The union of all validator types that can be accepted by a ControlConfig.
*/
type ValidatorConfig = ValidatorFn|AsyncValidatorFn|ValidatorFn[]|AsyncValidatorFn[];

/**
* The compiler may not always be able to prove that the elements of the control config are a tuple
* (i.e. occur in a fixed order). This slightly looser type is used for inference, to catch cases
* where the compiler cannot prove order and position.
*
* For example, consider the simple case `fb.group({foo: ['bar', Validators.required]})`. The
* compiler will infer this as an array, not as a tuple.
*/
type PermissiveControlConfig<T> = Array<T|FormControlState<T>|ValidatorConfig>;

/**
* ControlConfig<T> is a tuple containing a value of type T, plus optional validators and async
* validators.
Expand All @@ -31,7 +46,6 @@ function isAbstractControlOptions(options: AbstractControlOptions|{[key: string]
*/
export type ControlConfig<T> = [T|FormControlState<T>, (ValidatorFn|(ValidatorFn[]))?, (AsyncValidatorFn|AsyncValidatorFn[])?];


// Disable clang-format to produce clearer formatting for this multiline type.
// clang-format off

Expand Down Expand Up @@ -68,12 +82,7 @@ export type ɵElement<T, N extends null> =
// 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
// members that might be arrays). Watch for ControlConfigs that might fall through.
[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.
[T] extends [PermissiveControlConfig<infer U>] ? FormControl<Exclude<U, ValidatorConfig>|N> :
FormControl<T|N>;

// clang-format on
Expand Down
46 changes: 46 additions & 0 deletions packages/forms/test/typed_integration_spec.ts
Expand Up @@ -1019,6 +1019,16 @@ describe('Typed Class', () => {
t1 = null as unknown as RawValueType;
}
});

it('with array values', () => {
const c = fb.control([1, 2, 3]);
{
type RawValueType = number[]|null;
let t: RawValueType = c.getRawValue();
let t1 = c.getRawValue();
t1 = null as unknown as RawValueType;
}
});
});

describe('should build FormGroups', () => {
Expand Down Expand Up @@ -1060,6 +1070,16 @@ describe('Typed Class', () => {
}
});

it('from objects with FormControlStates nested inside ControlConfigs', () => {
const c = fb.group({foo: [{value: 'bar', disabled: true}, 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 validators', () => {
const c = fb.group({foo: ['bar', Validators.required]});
{
Expand All @@ -1068,6 +1088,23 @@ describe('Typed Class', () => {
let t1 = c.controls;
t1 = null as unknown as ControlsType;
}

const c2 = fb.group({foo: [[1, 2, 3], Validators.required]});
{
type ControlsType = {foo: FormControl<number[]|null>};
let t: ControlsType = c2.controls;
let t1 = c2.controls;
t1 = null as unknown as ControlsType;
}
expect(c2.controls.foo.value).toEqual([1, 2, 3]);

const c3 = fb.group({foo: [null, Validators.required]});
{
type ControlsType = {foo: FormControl<null>};
let t: ControlsType = c3.controls;
let t1 = c3.controls;
t1 = null as unknown as ControlsType;
}
});

it('from objects with ControlConfigs and validator lists', () => {
Expand Down Expand Up @@ -1482,6 +1519,15 @@ describe('Typed Class', () => {
}
c.reset();
expect(c.value).toEqual({foo: 'bar'});

const c2 = fb.group({foo: [[1, 2, 3], Validators.required]});
{
type ControlsType = {foo: FormControl<number[]>};
let t: ControlsType = c2.controls;
let t1 = c2.controls;
t1 = null as unknown as ControlsType;
}
expect(c2.controls.foo.value).toEqual([1, 2, 3]);
});

it('from objects with ControlConfigs and validator lists', () => {
Expand Down

0 comments on commit b302797

Please sign in to comment.