Make `AbstractControl#value` a union of `T` and `ValidationErrors`

[yawaramin]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

AbstractControl#value currently has type any (after #20040, it will have generic type T). AbstractControl#errors has type ValidationErrors | null. Currently we have to manually check for validation errors. It's easy to forget this check.

Expected behavior

• Type ValidationErrors is redefined as interface ValidationErrors<T> {currentValue: T, validators: Set<string>}
• There is a type ValueOrErrors<T> defined as T | ValidationErrors<T>
• There is a corresponding namepace ValueOrErrors with predicate functions that test ValueOrErrors<T> values to determine their runtime subtype (T or ValidationErrors<T>)
• There is a property AbstractControl#valueOrErrors with type ValueOrErrors<T>
• AbstractControl#value is deprecated and later removed
• AbstractControl#errors is deprecated and later removed
• AbstractControl#valid is deprecated and later removed
• AbstractControl#invalid is deprecated and later removed

This forces the developer to check for errors in order to compile their code. It also decouples the concepts of a form control having a value/error from queries on the value/error type. E.g.:

if (ValueOrErrors.isValue(inputName.valueOrErrors))
  PersonService.add(inputName.valueOrErrors);
else
  console.log(inputName.valueOrErrors.validators);

Minimal reproduction of the problem with instructions

Example copied from https://angular.io/guide/form-validation :

ngOnInit(): void {
  this.heroForm = new FormGroup({
    'name': new FormControl(this.hero.name, [
      Validators.required, Validators.minLength(4)])
  });
}

// ... type in 'Bob' in the 'name' field ...

// Returns 'Bob' which hasn't passed the minimum length validation:
get name() { return this.heroForm.get('name'); }

What is the motivation / use case for changing the behavior?

Forms gather input from the untrusted outside world, hence it makes sense to make them as safe as possible. Recruiting the TypeScript compiler to help enforce safety checks is a big benefit.

Environment

Angular version: (all)


Browser:
- [x] Chrome (desktop) version XX
- [x] Chrome (Android) version XX
- [x] Chrome (iOS) version XX
- [x] Firefox version XX
- [x] Safari (desktop) version XX
- [x] Safari (iOS) version XX
- [x] IE version XX
- [x] Edge version XX
 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

Depends on: #20040

[Toxicable]

How will you tell the difference between a value and an error?
An AbstractControl#value can be anything from a primitive number | string | boolean to a complex object in the case of a FormGroup, the same type as the error object.
Therefore there is no reasonable or easy way to tell the difference between an error and an actual value.

[yawaramin]

@Toxicable if we make ValidationErrors<T> a nominal type, in other words constructed using an ES6-style class i.e. using a function constructor--in this case, would we ever have an AbstractControl<ValidationsErrors<T>>? (Again, this is not a JS structural object.) Is that something that can happen?

My guess is no, that would not make any sense, because FormGroup contained types are composed out of their child form control contained types, and a nominal ValidationErrors<T> can never result from such a composition.

We can test for nominal function-constructed types like this using instanceof, so it's possible to implement the type predicates.

[Toxicable]

A class instance here would work since you could do something like

submit(value: ValidationErrors | T){
  if(value instanceof ValidationErrors) {
     //handle errors
   } else {
    //submit
  }
}

However this brings more issues with it.

• it's the same amount of code we started with
• it's impossible to check for errors in templates since {{value instanceof ValidationErrors}} is invalid

Aside from those issues the larger issue here is the breaking change.
I don't see how we could make this an in place change, therefore we'd have to make a "ReactiveForms2" just for this change which I think is unacceptable

[yawaramin]

There isn't a managed process (i.e. deprecation warning, staged rollout) for breaking changes in Angular now?

Re: templates, OK, I haven't thought through that properly. Re: same amount of code, the main benefit I'm proposing is type safety i.e. not being able to process form input data without checking it first, not a reduction in code size.

[Toxicable]

The policy for breaking changes is that

• New API is introduced
• Old APi is marked as deprecated
• Once old API has been deprecated for the length of one Major version it is removed

Think of Renderer, it went through this process with Renderer2 being the new API.

The point here is that we cannot make a in place breaking change even on a major version bump, the API must go through the deprecation period

[yawaramin]

OK, by 'in-place' breaking change you are considering the entire AbstractControl class to be a single unit of change, I think? I was looking at it at the method level, hence deprecating some existing methods/getters and adding a new getter and a couple of new types seemed feasible.

Btw, I looked at templates and it seems it would be something like this:

<div *ngIf="ValueOrErrors.isErrors(name.valueOrErrors)">
  Errors!
</div>

It seems string interpolation is not used for checking validation at all ( https://angular.io/guide/form-validation )?

[Toxicable]

Even on a method level, that has to be done via a depreciation period, we cannot break peoples code without it.

ValueOrErrors.isErrors(name.valueOrErrors)

I have two issues with this

• it's a function call, which I very often discourage since it can easily lead to slow CD runs when there are side effects
• what is ValueOrErrors? Is it another thing we have to define on our component classes? it's just making it more difficult and less intuitive to find if a control has errors

[yawaramin]

I totally agree that even on a method level there has to be a deprecation period. You mentioned a period of at least one major release before the deprecated items are removed, I fully agree with that. This is why I arranged the change as adding the new getter at the same time as deprecating the old ones.

Regarding your two points:

• Correct, this is a function call, but it's also a type predicate, which merely checks the runtime type of some value. The implementation I have prototyped is totally side-effect free, and I wouldn't be surprised if most VMs just inline the function calls altogether.
• ValueOrErrors is a type and its associated namespace which I am proposing to add to Angular to represent a control's current value or errors if the input fails validation. We wouldn't redefine it in component classes--we would use it everywhere and it would surface validation errors more effectively since you'd have to call control.valueOrErrors to get at the control's current value, and when you looked up the API docs you'd see it's giving you back a result of type ValueOrErrors, and when you look up that type in the docs you'll see the type predicate functions which help you test whether you have a value or validation errors. In short, you would be forced to check for validation errors before you could access the control's value.

[angular-robot]

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

[angular-robot]

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.