Closed asfernandes closed 2 years ago
related #11279
This is not related to #11279.
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.
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.
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 (https://github.com/Microsoft/TypeScript/issues/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.
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.
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?
@desfero waiting on #16707 for the build to be upgraded to TS2.3
+1 would love to see this feature. Anybody working on it?
Currently blocked on https://github.com/Microsoft/TypeScript/issues/16229
This might be of use - Angular Typesafe Reactive Forms
Small update on this: As per my comment here: https://github.com/angular/angular/pull/16828#issuecomment-337034655 implementing generics into the current Forms API is not possible without breaking changes. So either breaking changes are needed Or a full forms rewrite
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
@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.
@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)
@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.
@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.
I think that this method of accessing other controls is not related to adding generics. Feel free to open another issue about it however
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.
Hi, I've released https://github.com/Quramy/ngx-typed-forms for workaround of this issue. Please check it out 😄
@Quramy I tried to use your package several weeks ago and as I remember, it doesn't really do a lot of enforcement :(
+1. Can't count amount of instances when I wished it was implemented.
Same. Angular Reactive forms is one the features that really beats any other framework out there. Making Reactive forms strongly typed would bring it to the next level, further widening the gap to the competition :)
this can be done with conditional mapped types and recursion..... conditional mapped types were just merged into typescript. If this will be published we have a chance to make strong typed forms possible
Can't wait(
@Quramy solution sadly does not enforce the structure of all arguments to FormBuilder
. Also generic FormGroup<T>
, FormControll<T>
and FormArray<T>
can not be used directly, because they are only interfaces which don't event extend AbtractControl<T>
. This was not sufficient for our current project.
With ngx-strongly-typed-forms I haven now released a strong typed forms project myself It breaks a bit with backwards compatibility, because it doesn't use default generics. So it forces you to explicity give your controls a type of any, but adds a lot more typesafety to all other parts and is API compatible with the current implementation of @angular/forms. Perhaps this is a valid alternativ until this feature gets implemented in Angular.
+1 This is a powerful feature..
Should be implemented ASAP)
The way it's meant to be coded
There's an interesting implementation in this open source software (AGPL)
https://github.com/concentricsky/badgr-ui/blob/master/src/app/common/util/typed-forms.ts
I used this as a temporary workaround: Hope it helps
import { FormControl, AbstractControl } from "@angular/forms";
export class NumberControl extends FormControl{
_value: Number;
get value(){
return this._value;
}
set value(value){
this._value = Number(value);
}
}
This is something I've thought about a few times in projects I've worked on, but I haven't used JavaScript proxies enough to know the performance impact this would have on anything observing these values.
I simply created a custom workaround at the FormBuilder level:
import { Injectable } from '@angular/core';
import { FormBuilder, FormGroup } from '@angular/forms';
type ExtraProperties = { [key: string]: any } | null;
interface IModelConstructor { new(...args: any[]): any }
@Injectable({
providedIn: 'root'
})
export class TypedFormBuilder {
array = this.formBuilder.array;
control = this.formBuilder.control;
group = this.formBuilder.group;
constructor(private formBuilder: FormBuilder) {}
typedGroup<TGroupModel extends object>(ModelType: IModelConstructor, extraProps?: ExtraProperties): FormGroup & TGroupModel {
const formGroup: any = this.group({
...new ModelType(),
...extraProps
});
return new Proxy<FormGroup & TGroupModel>(formGroup, {
get: (target: FormGroup & TGroupModel, propName: string | number | symbol) => {
if (propName in target) {
return target[propName];
} else {
const property = target.get(propName as string);
return property && property.value;
}
},
set: (target: FormGroup & TGroupModel, propName: string | number | symbol, value: any, _: any) => {
if (propName in target) {
target[propName] = value;
} else {
target.setValue({ [propName]: value });
}
return true;
}
});
}
}
This solution is definitely not super polished, and it probably would be best to have the FormGroup generated access the values on a property (such as myGroup.fields, where "fields" would be the provided type). But this does provide strong typing. To use it:
generateFormGroup() {
this.theGroup = builder.typedGroup<MyModel>(MyModel);
this.theGroup.someProperty = 5;
}
I gathered experience in the last months with typed forms, by using my forms typings in my current project. It provides great value when working on one project with three developer teams and everybody switched because it was really cheap to do so.
But I want to discuss, if it's the right decision just to put generics on the current API. While building the types for all form controls, I found a lot of edge cases and things that are impossible or unwieldy to type, because I think static typing was not possible at that point in time and so not one of the biggest concerns.
Sadly this targets the main functionality with AbstractControl#value
, which must be something like DeepPartial<T>
, or AbstractControl#get
with different implementations for each subclass.
Being backwards compatible also loses some of type safety caused by fall through cases with any
types.
Perhaps considering a new API for reactive forms is also an option for this problem?
So, this is what I ended up doing while an actual solution happens.
Disclaimer... I just started in Angular, but quite familiar with Typescript, so I do not fully understand reactive forms... here is what ended up working for me, but of course it's not fully complete, I just typed FormGroup, but I'm sure more thing will need to be typed as I learn more about the forms...
import { FormGroup } from '@angular/forms';
export class FormGroupTyped<T> extends FormGroup {
public get value(): T {
return this.value;
}
}
and then I can use it like this
import { FormGroupTyped } from 'path/to/your/form-group-typed.model';
interface IAuthForm {
email: string;
password: string;
}
const authForm: FormGroupTyped<IAuthForm> = fb.group({
email: ['', [Validators.required]],
password: ['', [Validators.required]],
});
const formValues: IAuthForm = this.authForm.value;
const email: string = formValues.email;
const invalidKeyVar: string = formValues.badBoy; // [ts] Property 'badBoy' does not exist on type 'IAuthForm'. [2339]
const invalidTypeVar: number = formValues.password; // [ts] Type 'string' is not assignable to type 'number'. [2322]
@Toxicable Lol was searching this exact problem! haha
@cafesanu Here is a little improvement of your typed FormGroup to check the constructor.
import { AbstractControl, AbstractControlOptions, AsyncValidatorFn, FormGroup, ValidatorFn } from '@angular/forms';
export class FormGroupTyped<T> extends FormGroup {
readonly value: T;
readonly valueChanges: Observable<T>;
constructor(controls: { [key in keyof T]: AbstractControl; },
validatorOrOpts?: ValidatorFn | Array<ValidatorFn> | AbstractControlOptions | null,
asyncValidator?: AsyncValidatorFn | Array<AsyncValidatorFn> | null) {
super(controls, validatorOrOpts, asyncValidator);
}
patchValue(value: Partial<T> | T, options?: {
onlySelf?: boolean;
emitEvent?: boolean;
}): void {
super.patchValue(value, options);
}
get(path: Array<Extract<keyof T, string>> | Extract<keyof T, string>): AbstractControl | never {
return super.get(path);
}
}
usage :
export class SearchModel {
criteria: string;
}
const searchForm = new FormGroupTyped<SearchModel >({
criteria: new FormControl('', Validators.required) // OK
badBoy: new FormControl('', Validators.required) // TS2345: Object literal may only specify known properties, and 'badBoy' does not exist in type '{ criteria: AbstractControl; }'.
});
I wrote a little wrapper around FromControl that allows for dynamically generating the type of the data based on calls to a builder: https://github.com/concentricsky/badgr-ui/blob/develop/src/app/common/util/typed-forms.ts
The dynamically-constructed type ensure that the shape of the form matches your expectations, rather than having to pre-declare the interface type and then hope you make the right calls to create the form.
I'd love to see something similar to this in Angular at some point.
The usage looks like this:
// Create a typed form whose type is dynamically constructed by the builder calls
const group = new TypedFormGroup()
.add("firstName", typedControl("", Validators.required))
.add("lastName", typedControl("", Validators.required))
.add(
"address",
typedGroup()
.add("street", typedControl("2557 Kincaid"))
.add("city", typedControl("Eugene"))
.add("zip", typedControl("97405"))
)
.addArray(
"items",
typedGroup()
.addControl("itemName", "")
.addControl("itemId", 0)
)
;
// All these are type checked:
group.value.address.street.trim();
group.controls.firstName.value;
group.untypedControls.firstName.value;
group.value.items[0].itemId;
Hi all in the last 3 days I was experimenting using a d.ts gist
to define a more strit-ier definition for ReactiveForms classes creating new Typed interface compatible with original angular class.
I think that it may be a possible solution/workaround for your problem 😉
//BASIC TYPES DEFINED IN @angular/forms + rxjs/Observable
type FormGroup = import("@angular/forms").FormGroup;
type FormArray = import("@angular/forms").FormArray;
type FormControl = import("@angular/forms").FormControl;
type AbstractControl = import("@angular/forms").AbstractControl;
type Observable<T> = import("rxjs").Observable<T>;
type STATUS = "VALID" | "INVALID" | "PENDING" | "DISABLED"; //<- I don't know why Angular Team doesn't define it https://github.com/angular/angular/blob/7.2.7/packages/forms/src/model.ts#L15-L45)
type STATUSs = STATUS | string; //<- string is added only becouse Angular base class use string insted of union type https://github.com/angular/angular/blob/7.2.7/packages/forms/src/model.ts#L196)
//OVVERRIDE TYPES WITH STRICT TYPED INTERFACES + SOME TYPE TRICKS TO COMPOSE INTERFACE (https://github.com/Microsoft/TypeScript/issues/16936)
interface AbstractControlTyped<T> extends AbstractControl {
// BASE PROPS AND METHODS COMMON TO ALL FormControl/FormGroup/FormArray
readonly value: T;
valueChanges: Observable<T>;
readonly status: STATUSs;
statusChanges: Observable<STATUS>;
get<V = unknown>(path: Array<string | number> | string): AbstractControlTyped<V> | null;
setValue<V>(value: V extends T ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
patchValue<V>(value: V extends Partial<T> ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
reset<V>(value?: V extends Partial<T> ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
}
interface FormControlTyped<T> extends FormControl {
// COPIED FROM AbstractControlTyped<T> BECOUSE TS NOT SUPPORT MULPILE extends FormControl, AbstractControlTyped<T>
readonly value: T;
valueChanges: Observable<T>;
readonly status: STATUSs;
statusChanges: Observable<STATUS>;
get<V = unknown>(path: Array<string | number> | string): AbstractControlTyped<V> | null;
setValue<V>(value: V extends T ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
patchValue<V>(value: V extends Partial<T> ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
reset<V>(value?: V extends Partial<T> ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
}
interface FormGroupTyped<T> extends FormGroup {
// PROPS AND METHODS SPECIFIC OF FormGroup
//controls: { [P in keyof T | string]: AbstractControlTyped<P extends keyof T ? T[P] : any> };
controls: { [P in keyof T]: AbstractControlTyped<T[P]> };
registerControl<P extends keyof T>(name: P, control: AbstractControlTyped<T[P]>): AbstractControlTyped<T[P]>;
registerControl<V = any>(name: string, control: AbstractControlTyped<V>): AbstractControlTyped<V>;
addControl<P extends keyof T>(name: P, control: AbstractControlTyped<T[P]>): void;
addControl<V = any>(name: string, control: AbstractControlTyped<V>): void;
removeControl(name: keyof T): void;
removeControl(name: string): void;
setControl<P extends keyof T>(name: P, control: AbstractControlTyped<T[P]>): void;
setControl<V = any>(name: string, control: AbstractControlTyped<V>): void;
contains(name: keyof T): boolean;
contains(name: string): boolean;
get<P extends keyof T>(path: P): AbstractControlTyped<T[P]>;
getRawValue(): T & { [disabledProp in string | number]: any };
// COPIED FROM AbstractControlTyped<T> BECOUSE TS NOT SUPPORT MULPILE extends FormGroup, AbstractControlTyped<T>
readonly value: T;
valueChanges: Observable<T>;
readonly status: STATUSs;
statusChanges: Observable<STATUS>;
get<V = unknown>(path: Array<string | number> | string): AbstractControlTyped<V> | null;
setValue<V>(value: V extends T ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
patchValue<V>(value: V extends Partial<T> ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
reset<V>(value?: V extends Partial<T> ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
}
interface FormArrayTyped<T> extends FormArray {
// PROPS AND METHODS SPECIFIC OF FormGroup
controls: AbstractControlTyped<T>[];
at(index: number): AbstractControlTyped<T>;
push<V = T>(ctrl: AbstractControlTyped<V>): void;
insert<V = T>(index: number, control: AbstractControlTyped<V>): void;
setControl<V = T>(index: number, control: AbstractControlTyped<V>): void;
getRawValue(): T[];
// COPIED FROM AbstractControlTyped<T[]> BECOUSE TS NOT SUPPORT MULPILE extends FormArray, AbastractControlTyped<T[]>
readonly value: T[];
valueChanges: Observable<T[]>;
readonly status: STATUSs;
statusChanges: Observable<STATUS>;
get<V = unknown>(path: Array<string | number> | string): AbstractControlTyped<V> | null;
setValue<V>(value: V extends T[] ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
patchValue<V>(value: V extends Partial<T>[] ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
reset<V>(value?: V extends Partial<T>[] ? V : never, options?: { onlySelf?: boolean; emitEvent?: boolean }): void;
}
Usage testing in stackblitz - please download code and run in local VSCode I don't know why on stackblitz the ERRORS for setValue and pathValue are not correct...
On this twitter thread I discuss with @IgorMinar of some "future ideas" (after V8+)
Any comments, suggestions and help are very welcome!
My solution called @ng-stack/forms
. One of the interesting features is the automatic detect of form types.
So, no need to do this in your components:
get userName() {
return this.formGroup.get('userName') as FormControl;
}
get addresses() {
return this.formGroup.get('addresses') as FormGroup;
}
Now do this:
// Note here form model UserForm
formGroup: FormGroup<UserForm>;
get userName() {
return this.formGroup.get('userName');
}
get addresses() {
return this.formGroup.get('addresses');
}
For more info, see @ng-stack/forms
Hey, we've been working with @zakhenry recently on 2 things:
We do not cover all the types as @dmorosinotto did but we've got some safety that'd definitely help when making refactors (for both ts and html :tada:).
If anyone wants to take a look to the lib: ngx-sub-form
Hi @maxime1992 I've looked at the Controls<T>
but it doesn't strict the AbstractControl to <T[K]>
Can I ask you why? Do you leave it "untyped" because you feel the needs to change the type at run-time using something like setControl or registerControl methods to redefines and changes the FormControl and maybe the associated type?
Because my TypedForms.d.ts is a bit stricter and "force" the AbstractControlTyped to the type T<P>
but I don't know if with this kind of choice enforce / disable something that in the original ReactiveForms API is allowed and maybe used by someone...
Any thought? Any real-case to consider? Any comments on this may help me to decide on how to change the definitions that I've created and the PR that I'm working on... Thanks
PS: Great work on ngx-sub-form 👍 the idea to use ControlValueAccessor to handle subforms is something I was experimenting too 😉
For @ng-stack/forms added support typed validation.
Classes FormControl
, FormGroup
, FormArray
and all methods of FormBuilder
accept "error validation model" as second parameter for a generic:
class ValidationModel {
someErrorCode: { returnedValue: 123 };
}
const control = new FormControl<string, ValidationModel>('some value');
control.getError('someErrorCode'); // OK
control.errors.someErrorCode; // OK
control.getError('notExistingErrorCode'); // Error: Argument of type '"notExistingErrorCode"' is not...
control.errors.notExistingErrorCode; // Error: Property 'notExistingErrorCode' does not exist...
By default used special type called ValidatorsModel
.
const control = new FormControl('some value');
control.getError('required'); // OK
control.getError('email'); // OK
control.errors.required // OK
control.errors.email // OK
control.getError('notExistingErrorCode'); // Error: Argument of type '"notExistingErrorCode"' is not...
control.errors.notExistingErrorCode // Error: Property 'notExistingErrorCode' does not exist...
ValidatorsModel
contains a list of properties extracted from typeof Validators
, and expected returns types:
class ValidatorsModel {
min: { min: { min: number; actual: number } };
max: { max: { max: number; actual: number } };
required: { required: true };
requiredTrue: { required: true };
email: { email: true };
minLength: { minlength: { requiredLength: number; actualLength: number } };
maxLength: { requiredLength: number; actualLength: number };
pattern: { requiredPattern: string; actualValue: string };
}
@dmorosinotto Thank you so much but I am not able to use
Validators.compose([Validators.required, Validators.maxLength(20), Validators.minLength(3)])
Hi @youssefsharief can you give me more details of your problem with Validators? What kind of error/problem did you find using the .d.ts?
If you can post a sample code or a stackblitz I'll watch at it and I'll try to help solve the problem if I found a solution 😉
For @ng-stack/forms
added support for input[type="file"]
.
See also example on stackblitz
I also used "classic" approach with using FormData
to upload files, but there are several profits:
<input type="file" name="someName">
-> formControl.value
with someName
field name;multiple
attribute with properly setting field name (note userpic[]
here)
<input type="file" multiple name="userpic" [formControl]="formControl">
formData.append('userpic[]', myFileInput.files[0]);
formData.append('userpic[]', myFileInput.files[1]);
Validators
have four static methods for files
import { Validators } from '@ng-stack/forms';
Validators.fileRequired; Validators.fileMaxSize(1024); Validators.filesMaxLength(10); Validators.filesMinLength(2);
@KostyaTretyak from what I saw so far your package seems to be a great solution for the current typing issues in Angular Reactive Forms. Ever thought about contributing to Angular itself instead of doing your own implementation?
We are almost in angular 8 and still have no out-of-the-box typed forms in a 100% TypeScript environment which feels pretty odd to me.
@kroeder, I just see that the current issue was created more than two years ago (almost immediately after the release of Angular 2), and I see that similar Pull Request was created 1.5 years ago without a merge...
But if @ng-stack/forms
will be popular and compatible with Angular 8+, maybe I will create the Pull Request in the future.
@KostyaTretyak Sounds great :) I've been using https://github.com/no0x9d/ngx-strongly-typed-forms which is mentioned earlier in this thread and created by @no0x9d How does your library differ from that?
@ZNS, I don't know, but after a quick review, I think there is no generic for validation in ngx-strongly-typed-forms
, as well as the Automatically detect appropriate types for form controls. Maybe I'm wrong.
@ZNS @KostyaTretyak Hi there. As mentioned above, I am the author of ngx-strongly-typed-forms.
I did a quick review of the feature set of @ng-stack/forms
and think there are some small differences.
There is a conceptional difference between nearly all solutions or workarounds and my project. In most cases the original Angular FormControl is extended or wraped by some Proxy. Some methods are overridden with other type signatures and delegated to the original function. This introduces a new layer with a negliable performance impact, but more important is code that has to be maintained and can introduce bugs to your project. In my opinion this is unnecessary for static checks at compile time. The only run time part is the NgModule that provides my typed FormBuilder, which is in fact the original Angular FormBuilder. Everything else is just Angular code.
In direct comparison I don't have the ValidationModel and conversion from objects to FormGroups and Arrays to FormArrays but did some opinionated changes to AbstractControl#get
to make it more type safe and have typed validator arguments.
With some small additions my code can be backwards compatible and I could create a pull request. But a simmilar pull request is stale for a long time. But if there are efforts to get this in Angular I would be happy to join forces. On the other side I would love to see a new API for forms that is better designed to be strictly typed. See my comment for details.
+1 would love to see this feature. Anybody working on it?
bump to angular team?
Reactive forms is meant to be used in complex forms but control's
valueChanges
areObservable<any>
, which are totally against good practices for complex code.There should be a way to create strongly typed form controls.