Skip to content

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

Closed
dylhunn wants to merge 1 commit intoangular:masterfrom
dylhunn:fr
Closed

fix(forms): Fix a typing bug in FormBuilder.#45684
dylhunn wants to merge 1 commit intoangular:masterfrom
dylhunn:fr

Conversation

@dylhunn
Copy link
Contributor

@dylhunn dylhunn commented Apr 19, 2022

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, so the inference would instead return FormControl<Array<string|ValidatorFn>>. 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.

@dylhunn dylhunn added action: review The PR is still awaiting reviews from at least one requested reviewer area: forms target: major This PR is targeted for the next major release forms: Controls API Issues related to AbstractControl, FormControl, FormGroup, FormArray. cross-cutting: types labels Apr 19, 2022
@ngbot ngbot bot added this to the Backlog milestone Apr 19, 2022
@dylhunn dylhunn force-pushed the fr branch 2 times, most recently from 737cca5 to b7b5b97 Compare April 19, 2022 20:23
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some minor comments 👍

Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-for: public-api

@dylhunn
Copy link
Contributor Author

dylhunn commented Apr 19, 2022

@cexbrayat Just FYI, this fixes the issue you reported. Thanks!

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.
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

@jessicajaniuk jessicajaniuk removed the request for review from alxhub April 20, 2022 12:13
@dylhunn dylhunn added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 20, 2022
@dylhunn
Copy link
Contributor Author

dylhunn commented Apr 20, 2022

This PR was merged into the repository by commit ff3f5a8.

@dylhunn dylhunn closed this in ff3f5a8 Apr 20, 2022
atscott pushed a commit to atscott/angular that referenced this pull request Apr 20, 2022
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 angular#45684
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 10, 2022
@dylhunn dylhunn deleted the fr branch November 30, 2022 20:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: forms cross-cutting: types forms: Controls API Issues related to AbstractControl, FormControl, FormGroup, FormArray. forms: strictly typed target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants