The Wayback Machine - https://web.archive.org/web/20220611175927/https://github.com/angular/angular/pull/43834
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(forms): Implement strict types for the Angular Forms package. #43834

Closed
wants to merge 1 commit into from

Conversation

dylhunn
Copy link
Contributor

@dylhunn dylhunn commented Oct 14, 2021

This PR strongly types the forms package by adding generics to AbstractControl classes as well as FormBuilder. This makes forms type-safe and null-safe, for both controls and values.

The design uses a "control-types" approach. In other words, the type parameter on FormGroup is an object containing controls, and the type parameter on FormArray is a control.

Issue: #13721

@google-cla google-cla bot added the cla: yes label Oct 14, 2021
@dylhunn dylhunn requested review from AndrewKushnir and alxhub Oct 14, 2021
@dylhunn dylhunn added comp: forms forms: Controls API labels Oct 14, 2021
@ngbot ngbot bot added this to the Backlog milestone Oct 14, 2021
@ngbot ngbot bot added this to the Backlog milestone Oct 14, 2021
@dylhunn dylhunn added action: discuss breaking changes design complexity: major discussion risk: high risky state: WIP target: major labels Oct 14, 2021
@dylhunn dylhunn removed this from the Backlog milestone Oct 14, 2021
@dylhunn dylhunn added this to the v14-candidates milestone Oct 14, 2021
@NetanelBasal
Copy link

@NetanelBasal NetanelBasal commented Oct 14, 2021

There are a lot of challenges in this area due to the flexibility of the API. As far as I can tell, I've written a typed version that reaches the maximum we can without breaking the API. Feel free to look at the source code:

https://github.com/ngneat/reactive-forms

Furthermore, it would be helpful to add specs for the types in this case. You can see an example here:

https://github.com/ngneat/reactive-forms/blob/master/libs/reactive-forms/src/lib/form-group.spec.ts#L333

@dylhunn
Copy link
Contributor Author

@dylhunn dylhunn commented Oct 14, 2021

I've written a typed version that reaches the maximum we can without breaking the API

Wonderful, somehow I wasn't aware of this. I'll be sure to have a look.

@dylhunn dylhunn force-pushed the typed-forms-default branch 2 times, most recently from 2d42212 to d99a19e Compare Oct 15, 2021
@dylhunn dylhunn force-pushed the typed-forms-default branch 2 times, most recently from 9cfa76c to 5cc693e Compare Nov 3, 2021
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

FYI, just sharing first batch of comments, will publish more comments tomorrow.

packages/forms/src/directives/validators.ts Outdated Show resolved Hide resolved
packages/forms/src/form_builder.ts Show resolved Hide resolved
packages/forms/src/form_builder.ts Outdated Show resolved Hide resolved
packages/forms/src/form_builder.ts Outdated Show resolved Hide resolved
packages/forms/src/form_builder.ts Outdated Show resolved Hide resolved
packages/forms/src/form_builder.ts Outdated Show resolved Hide resolved
packages/forms/src/model.ts Outdated Show resolved Hide resolved
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Apr 8, 2022

@angular angular deleted a comment from mary-poppins Apr 8, 2022
@dylhunn dylhunn force-pushed the typed-forms-default branch 4 times, most recently from bcce3cf to 7959624 Compare Apr 8, 2022
@dylhunn dylhunn force-pushed the typed-forms-default branch 3 times, most recently from 034ddce to 52f9aaa Compare Apr 11, 2022
@dylhunn
Copy link
Contributor Author

@dylhunn dylhunn commented Apr 11, 2022

TGP is green, with two final fixup CLs patched: one, two

merge-assistance: merge separately, Tuesday morning

@dylhunn dylhunn added action: merge-assistance and removed action: presubmit labels Apr 11, 2022
@ngbot ngbot bot added the action: merge label Apr 11, 2022
This PR strongly types the forms package by adding generics to AbstractControl classes as well as FormBuilder. This makes forms type-safe and null-safe, for both controls and values.

The design uses a "control-types" approach. In other words, the type parameter on FormGroup is an object containing controls, and the type parameter on FormArray is an array of controls.

Special thanks to Alex Rickabaugh and Andrew Kushnir for co-design & implementation, to Sonu Kapoor and Netanel Basal for illustrative prior art, and to Cédric Exbrayat for extensive testing and validation.

BREAKING CHANGE: Forms classes accept a generic.

Forms model classes now accept a generic type parameter. Untyped versions of these classes are available to opt-out of the new, stricter behavior.
@dylhunn dylhunn added action: merge-assistance and removed action: merge action: merge-assistance labels Apr 12, 2022
@ngbot ngbot bot added the action: merge label Apr 12, 2022
@jessicajaniuk
Copy link
Contributor

@jessicajaniuk jessicajaniuk commented Apr 12, 2022

This PR was merged into the repository by commit 89d2991.

@depyronick
Copy link

@depyronick depyronick commented Apr 13, 2022

it's been 5+ years of wait but worth it. thanks everybody!

@angular-automatic-lock-bot
Copy link

@angular-automatic-lock-bot angular-automatic-lock-bot bot commented Jun 10, 2022

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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.