Uninitialized FormControl can generate runtime errors

[benjamin-thomas]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[x ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

Hello, neat library ;) And looking to be a great time saver

I'm puzzled by the possibility of initializing a FormControl with bad data. Please see the following screenshots:

Here, I forgot to initialize the activityName form control with a value.

Later, I feed this form control to a function that requires a string:

The compiler doesn't have my back unfortunately, and I get a runtime error!

Expected behavior

I would expect to have as much type safety as possible at compile time.

I'm not familiar with writing generics producing code. But a quick prototype shows me it is possible to add this constraint at the language level.

Is this a bug? I can't se a valid use case for the current behavior.

I can see you mark the formState as optional here:

reactive-forms/projects/ngneat/reactive-forms/src/lib/formControl.ts

Line 70 in 4c886dc

constructor(formState?: OrBoxedValue<T>, validatorOrOpts?: ValidatorOrOpts, asyncValidator?: AsyncValidator) {

If that's really a necessity, I would expect the underlying form control value to return a string | undefined (which would still be annoying be better than getting a runtime error)

Thanks :)

[NetanelBasal]

We mimic the same functionality of Angular.

If that's really a necessity, I would expect the underlying form control value to return a string | undefined

You can add it: FormControl<string | undefined>

[benjamin-thomas]

Thanks. I would argue this doesn't follow the principle of least surprise. I feel the type system is lying to me at this point.

Regarding Angular, I understand you want to mimic it's functionality but this one is bug inducing. Also by introducing type safety you have already improved and changed the default functionality, so I feel you could go one step further.

Maybe the following screenshot will make you reconsider this design decision. Otherwise please feel free to close ;)

[NetanelBasal]

Don't get me wrong. I agree with you. But the focus of this library, at least for step one, is providing a seamless migration. I'll leave it open to v2.

[benjamin-thomas]

Sounds good, thanks ;)

Have a great Sunday.

[benjamin-thomas]

Just a "food for thought" update, I think I got this backwards. Or at least there are other gotchas to be aware of.

Angular differs in behavior depending on the form input type:

• user empties an <input type="text"> field: the output is an empty string
• user empties an <input type="number"> field: the output is a null value!

That kind of behavior is not immediately obvious to me. And I can only imagine you have to plug into the Angular behavior at some point...

So to sum up, it may be a good idea to force the developer to handle the null case by forcing him to declare the nullability at the initialization stage,as such:

FormControl<number>  // => Cannot be true, generate some kind error
FormControl<number | null>  // => Ok

A runtime error on initializing would be fine since you'd make the mistake once, fix it then move on.
Since there's no way to enforce default values for a given type in TS unless I'm mistaken.

[NetanelBasal]

I am closing this issue as I released v3.