-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
fix(forms): allow control state as reset argument for FormGroup
#55860
base: main
Are you sure you want to change the base?
Conversation
319e8c2
to
46312f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my suggestions.
23aa90b
to
590481c
Compare
590481c
to
8fdb80f
Compare
Thank for the changes Walid ! |
We're investigating an issue illustrated by the following snippet: function valueChangesWrapper<T>(control: AbstractControl<T>) {
return control.valueChanges;
}
const fg = new FormGroup({foo: new FormControl('')});
fg.valueChanges;
const a = valueChangesWrapper(fg); Notice that the inferred type of This is super tricky to debug/understand. |
8fdb80f
to
eeab1c7
Compare
I've allowed myself to push the changes to fix the issues that were brought up by testing the fix against Google's code base. Basically what happens is that changing the reset() signature had implicationg on the |
eeab1c7
to
144f0cf
Compare
FormGroup
144f0cf
to
6bfaa7c
Compare
…rmRecord` This change also decorelate the `reset` type argument from `TValue` by adding a 3rd generic parameter to `AbstractControl`. This improves the typings overall.
6bfaa7c
to
51f080c
Compare
fix(forms): Allow ControlState as reset arguments for
FormGroup
/FormRecord
This change also decorelate the
reset
type argument fromTValue
by adding a 3rd generic parameter toAbstractControl
.This improves the typings overall.
This PR closes #55577.