Reactive forms are not strongly typed

[asfernandes]

[x] feature request

• Angular version: 2

Reactive forms is meant to be used in complex forms but control's valueChanges are Observable<any>, which are totally against good practices for complex code.

There should be a way to create strongly typed form controls.

[Toxicable]

related #11279

[asfernandes]

This is not related to #11279.

[Toxicable]

Please explain how it is not related?
What you want is for Abstract Control to be Generic right? That is the only way that valueChanges can have a type that is not Observable<any> there would be no other way to infer the type.
Which is exactly what #5404 is asking, which means this is related to #11279
If there is another way this could be implemented without making AbstractControl a generic please explain that.

[asfernandes]

Using get<Type> as in #11279 is definitively wrong solution. If TypeScript had somethign like Java Unbounded Wildcard, get would use it, and not any. Maybe something can be done in the same manner with a empty interface?

There is also https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-1.html keyof. TypeScript 2.1 features may be very interesting to study to implement strongly typed form controls.

The way it is currently, unfortunately, I do not thing it's usable for large app and I need to design something on top of it.

[Toxicable]

I just noticed that in TS 2.2 (https://github.com/Microsoft/TypeScript/wiki/Roadmap#22-february-2017) that they have planned Default Generic Types (microsoft/TypeScript#2175) once we have this I think it might be a good idea to revisit this issue since we could make AbstractControl generic like AbstractControl<T = any> where the T is the type of the value returned by valueChanges which would be an Observable<T>. It would not be a good idea to do it currently since it would a massive breaking change but with default generics, unless I misunderstand them, it will not be a breaking change.

[Toxicable]

Small update on this, looks like Default Generics have been been moved to TS2.3 . So with the support of TS 2.1 by Angular with version 4.0 it should not be long after that before they're able to support TS 2.3 which is now when we should wait to revisit this.

[desfero]

TypeScript 2.3 with Default Generic Types is already here, do we have any plans when support for TS 2.3 in angular will be ready?

[Toxicable]

@desfero waiting on #16707 for the build to be upgraded to TS2.3

[kemmis]

+1 would love to see this feature. Anybody working on it?

[Toxicable]

#16828
Currently blocked on microsoft/TypeScript#16229

[rpbeukes]

This might be of use - Angular Typesafe Reactive Forms

[Toxicable]

Small update on this:
As per my comment here: #16828 (comment)
implementing generics into the current Forms API is not possible without breaking changes.
So either breaking changes are needed
Or a full forms rewrite

[Toxicable]

So turns out my previous comment was incorrect.
I was able to implement this on the current Forms API as you can see here #20040

[howiempt]

@Toxicable that still has the problem of lacking the ability to refactor safely. get('person') for example, is not really using the symbol itself. The example above, from @rpbeukes, has a retrofitted way of basically using the object symbol eg. get(obj.person) without using the string. That would be preferred over just having return types.

[Toxicable]

@howiempt

get('person') for example, is not really using the symbol itself

I have no idea what you mean by this, what symbol are you referring to here?
In my implementation you can do something like

let g = new FormGroup({
  'name': new FormControl('Toxicable'),
  'age': new FormControl(22),
})

g.get('name') //AbstractControl<string>
g.get('age') //AbstractControl<number>

get(obj.person) without using the string

This lacks the ability to traverse multiple FormGroups.
While my method is unable to infer the types in this scenario the idea of my PR is to add Generic types without breaking changes or introducing any new API's (aside from generics)

[howiempt]

@Toxicable I understand you're change is meant to not break things, not trying to criticize your solution. The other implementation (retrofitted) allows an actual property to be used rather than a string. By referencing the the field by string, if that property name changes, builds break, which for me isn't very safe. For example, changing the field name from 'name' to 'firstName', would break if I didn't change all the g.get('name') references. If I could do something like

class PersonDetails {
  name: string;
  age: number;
}
let g = new FormGroup<PersonDetails>({
  name: new FormControl('Toxicable'),
  age: new FormControl(22),
})

g.get(name) //AbstractControl<string>
g.get(age) //AbstractControl<number>

They would all be tight references. The retrofit solution does it in a slightly hacky way but does solve that problem as well.

[rpbeukes]

@Toxicable thanx for the PR. Looking forward using it :)

I do agree with @howiempt, if we can get something like this it would be first prize:

g.get(x => x.name) //AbstractControl

Again, I don't really know how feasible this is within the greater scope of things.
I trust your judgement.

Keep up the good work and thanx for the quick response.

[Toxicable]

I think that this method of accessing other controls is not related to adding generics.
Feel free to open another issue about it however

[howiempt]

I really don't think that having the return type set is really "strongly typed", seems like half of the implementation required, but it is a step in the right direction.

[Quramy]

Hi, I've released https://github.com/Quramy/ngx-typed-forms for workaround of this issue. Please check it out 😄

[nicu-chiciuc]

@Quramy I tried to use your package several weeks ago and as I remember, it doesn't really do a lot of enforcement :(

[dylhunn]

PR #43834 merged the new types, and is released in 14.0.0-next.12. The next step is to land FormRecord, and then fix any bugs that arise during the next and RC periods.

You can now try out typed forms by switching to the next channel: ng update @angular/cli @angular/core --next

[e-oz]

I've tried new typed forms (Angular 14.0.0-next.12, TS 4.6.3 and TS 4.7.beta), and right now I don't quite understand how to use types with them.
2 examples:

1. I have a FormGroup with the field "text", type is "string". To declare it, I need to write:
   form = new FormGroup<{ text: FormControl<string | null> }>({text: this.textCtrl});
   I expected it will be:
   form = new FormGroup<{ text: string }>({text: this.textCtrl});
   but even this is not allowed:
   form = new FormGroup<{ text: FormControl<string> }>({text: this.textCtrl});
   because "FormControl" should be replaced with "FormControl<string | null>".

2. I have a complicated structure of type T, for this structure I create a FormGroup. Right now it's impossible to just declare this:
   form = new FormGroup<T>({});
   TS requires to declare an AbstractControl for every field of that big complicated structure.

Is it how it's supposed to work?

[e-oz]

One more question: how to remove a control now?

const oldAttrCtrls = this.attrsForm?.controls ? Object.keys(this.attrsForm.controls) : [];
if (oldAttrCtrls?.length) {
  oldAttrCtrls.forEach((attr) => {
    // 👇
    this.attrsForm.removeControl(attr); // 🛑 TS2769: No overload matches this call.  ... (long explanation here, because this form has a complicated type)
   //  👆
  });
}

The only workaround I've found is // @ts-ignore

[e-oz]

One more thing:
formGroup.get(someStringValue) still returns an AbstractControl, even if formGroup is fully typed (so no AbstractControls are expected). So in templates $any() is still needed (it's sad).

[e-oz]

Sorry for posting a lot, but one more thing: TypeScript goes crazy when it has to parse code with some typed FormGroup. CPU skyrockets. It's just my wild assumption, but looks like there are some endless recursions.

[fen89]

One more question: how to remove a control now?

const oldAttrCtrls = this.attrsForm?.controls ? Object.keys(this.attrsForm.controls) : [];
if (oldAttrCtrls?.length) {
  oldAttrCtrls.forEach((attr) => {
    // 👇
    this.attrsForm.removeControl(attr); // 🛑 TS2769: No overload matches this call.  ... (long explanation here, because this form has a complicated type)
   //  👆
  });
}

The only workaround I've found is // @ts-ignore

@e-oz Given your form consists of the following:

interface Person {
  firstName: string;
  lastName: string;
}

const form = this.fb.group<IPerson>({
  firstName: '',
  lastName: '',
});

Typescript is complaining that the argument given of type string is not assignable to any of your forms parameters => Argument of type 'string' is not assignable to parameter of type '"firstName" | "lastName"'.

1.) Casting the function to type any

form.controls.forEach((attr) => {
  form.removeControl<any>(attr); 
});

2.) Casting the string attr to keyof Person

form.controls.forEach((attr: keyof Person) => {
  form.removeControl(attr);
});

[e-oz]

Thanks, @fen89 , but my form has type Record<string, ComplicatedTypeHere>, so it should have string keys.

[dylhunn]

@e-oz

form = new FormGroup<{ text: string }>

We are using control types -- rely on the inference, or try FormGroup<{text: FormControl<string>}>.

"FormControl" should be replaced with "FormControl<string | null>".

You need to add the following option to your constructor: {initialValueIsDefault: true}.

TS requires to declare an AbstractControl for every field of that big complicated structure.

This is a use case for FormRecord as proposed in the RFC! FormRecord isn't quite ready yet, but it should land sometime next week -- it will support dynamic groups with homogenous keys and values, i.e. exactly the use case you're requesting.

how to remove a control now?

Declare the control as optional (FormGroup<{foo?: FormControl<string>}>) or use FormRecord (once it's available soon!)

formGroup.get(someStringValue) still returns an AbstractControl

This one is tricky to fix, unfortunately. I'm working on it, I'll post back with updates.

[dylhunn]

but my form has type Record<string, ComplicatedTypeHere>

@e-oz This is a perfect use case for FormRecord, stay tuned a few more days :)

(You could also use an index signature: FormGroup<{[key: string]: ComplexTypeWithControls}>, which does the same thing)

[e-oz]

More updates from me.

With the time I've found ways how to use new forms more efficiently, and I can't imagine how sweet it will be with FormRecord, because it's pretty sweet already right now :) Thank you very much for implementing this feature!

1. The most handy thing is the ability to use [formControl]="form.controls.someName" in the templates - it's so good, you need to try it.

2. Issue with CPU skyrocketing is localized, and it was caused by the use of "removeControl"/"addControl". It can be resolved pretty simple: just downcast form's type to <any>:

CPU-heating version:

const oldAttrCtrls = this.attrsForm?.controls ? Object.keys(this.attrsForm.controls) : [];
if (oldAttrCtrls?.length) {
  oldAttrCtrls.forEach((attr) => {
    // @ts-ignore
    this.attrsForm.removeControl(attr);
  });
}

CPU-polite version:

const form = this.attrsForm as FormGroup<any>; // temporary trick
const oldAttrCtrls = form?.controls ? Object.keys(form.controls) : [];
if (oldAttrCtrls?.length) {
  oldAttrCtrls.forEach((attr) => {
    form.removeControl(attr);
  });
}

Thanks again, I love to use new forms :)

[dylhunn]

@e-oz Are you using IntelliJ/WebStorm? The crazy CPU usage is caused by a bug in WebStorm's TypeScript integration (which is not the standard language server; it's unique to WebStorm). We are working with Jetbrains currently, and hope to have it resolved soon.

If you're using another IDE, that's concerning and I'd love to hear details.

[e-oz]

@dylhunn I use IntelliJ IDEA (not Webstorm, but from the same family). I've checked the code in VS Code and it works fine ("TypeScript: Tsdk" setting is set to "./node_modules/typescript/lib").

[dylhunn]

The performance issues with Jetbrains IDEs (Webstorm, IntelliJ) should be fixed in version 2022.1.1. Let us know if any issues are still occurring at that version or later.

[gparlakov]

Hey all, here's a drop-in alternative while waiting for this issue to be resolved.

npm package: ngx-forms-typed - https://www.npmjs.com/package/ngx-forms-typed

[dylhunn]

Hi everyone! I'm thrilled to say that typed reactive forms will be landing today in Angular 14. If you discover any bugs, please open a new issue to report them. Although there are some more improvements we'd like to make in the future, such as changes to template type checking, we will track those on a different issue.

[kasir-barati]

Official doc about this fantastic feature

When I came to this issue I tried to find the documentation and so I thought it is good to add it at the end of this discussion for those who wanted to immediately heave into it.

https://angular.io/guide/typed-forms

[thw0rted]

@kasir-barati I'm finally getting around to doing this upgrade and I found a mismatch between the docs and the actual behavior. I think an API change is probably in order, but if not, at least the docs should be updated to match behavior.

The docs say that new FormControl("some string")

will be automatically inferred to have the type FormControl<string|null>

This is not true, the type is inferred as FormControl<string>, but the base type (AbstractControl) is <T|null>, so myControl.value has type string|null. This also means that new FormControl<string>(null) is not allowed, even though myControl.reset() will set the value to null, if nonNullable is not true.

I think the constructor for FormControl should look like

constructor(initialValue: T, opts: {nonNullable: true})
constructor(initialValue: T|null)
constructor(initialValue: T|null, opts?: {nonNullable: true})

though I'm not sure what class generic param will be inferred for new FormControl(null) -- I assume null or undefined, maybe never? Without this code change, everybody will be stuck writing new FormControl(null as string | null), which is needlessly verbose.

[MartinJohns]

though I'm not sure what class generic param will be inferred for new FormControl(null) -- I assume null or undefined, maybe never?

@thw0rted It'll be null with strictNullChecks, and any without.

[SmallhillCZ]

Hi does this mean that ControlValueAccessor type safety in #19329, #19340, #31801 and similar, which were dismissed as a duplicate of this one were not addressed in the end?

[angular-automatic-lock-bot]

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.}