angular: Forms: State that setErrors() will make status === INVALID regardless of value passed for key

I’m submitting a…


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Feature request
[x] 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

In the setErrors example for AbstractControl it shows passing true for the key. The issue is that the control’s status will be set to INVALID no matter the value of the key (or even just passing setErrors an empty object (see _calculateStatus code) However, as the example is given, it’s reasonable to infer that you clear the errors by setErrors( {"notUnique": false}), when really you’d need to do setErrors(null) or, as the example does show, set the control’s value to a new value.

const login = new FormControl("someLogin");
login.setErrors({
  "notUnique": true
});

expect(login.valid).toEqual(false);
expect(login.errors).toEqual({"notUnique": true});

login.setValue("someOtherLogin");

expect(login.valid).toEqual(true);

Expected behavior

Update documentation to explicitly say how to clear errors in code (as this is useful to know when doing integration (template) testing, where you just want to set an error, check the template, then clear the error and check that any alerts have gone away.

Minimal reproduction of the problem with instructions

Failing test

const login = new FormControl("someLogin");
login.setErrors({
  "notUnique": true
});

expect(login.valid).toEqual(false);
expect(login.errors).toEqual({"notUnique": true});

login.setErrors({"notUnique": false});
expect(login.valid).toEqual(true); //will still be INVALID since the check: if(this.errors) will return TRUE

Suggested example

const login = new FormControl("someLogin");
login.setErrors({
  "notUnique": true
});

expect(login.valid).toEqual(false);
expect(login.errors).toEqual({"notUnique": true});

login.setValue("someOtherLogin"); //or: login.setErrors(null); <===add this comment

expect(login.valid).toEqual(true);

Alternatively (though this would be code change, not just a documentation change), is that in _calculateStatus(), you only set to INVALID if at least one key in the errors object is set to true.

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

When I am doing integration (template) testing to validate that alert/error messages show up when a control is invalid, and I choose to use the setErrors() function to create an INVALID status, I want to know how to then create a VALID status (either by setting a valid value or by passing null to the control.setErrors() function.

Environment


Angular version: 4.4.1


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

Others:

About this issue

  • Original URL
  • State: open
  • Created 6 years ago
  • Reactions: 18
  • Comments: 31 (9 by maintainers)

Most upvoted comments

I am using the following Utils methods to handle adding and removing errors from controls:

  public static removeErrors(keys: string[], control: AbstractControl) {
    if (!control || !keys || keys.length === 0) {
      return;
    }

    const remainingErrors = keys.reduce((errors, key) => {
      delete errors[key];
      return errors;
    }, { ...control.errors });

    control.setErrors(remainingErrors);

    if (Object.keys(control.errors || {}).length === 0) {
      control.setErrors(null);
    }
  }


  public static addErrors(errors: { [key: string]: any }, control: AbstractControl) {
    if (!control || !errors) {
      return;
    }

    control.setErrors({ ...control.errors, ...errors });
  }

I have come across the same problem in a different scenario.

I am using custom errors to set errors returned from the server on specific inputs. Only the server knows how to validate the values and the only feedback I get from the server comes in the error response after I POST something, so I can’t validate the values before attempting to POST. So when the server returns a status 400 I parse the response, I see which inputs have failed and then call setErrors on the inputs as appropiate.

After the user makes changes to the form I want to clear these errors, but if I call setErrors with null I’m also clearing other errors added by the validators I already have on some of the inputs.

I only want to remove the errors I added, not all the errors on that input. I think being able to add custom errors but not being able to remove them easily is not very useful.

I also hit this issue. I found it weird that setErrors(null) is the only way to properly clear the invalid state. Validators returning {'key': null} to reset an error state do not get applied, even if no other truthy keys are present.

I came from this stackoverflow question and try this little workaround 😬

Using Destructuring assignment

if ( control.hasError('errorToRemove') ) { // this validation is optional, I think
  const { errorToRemove, ...errors } = control.errors;
  control.setErrors(errors);
  control.updateValueAndValidity();
}

You can also try the omit or pick lodash functions, to respectively omit or pick a series of errors that you can set to the control later.

const errors = pick(control.errors, ['onlyErrorsToKeep']);
control.setErrors(errors);
control.updateValueAndValidity();
const errors = omit(control.errors, ['allErrorsToRemove']);
control.setErrors(errors);
control.updateValueAndValidity();

Given the errors value is not boolean-based indicator, but whatever information about this error going being exposed to user, setErrors should not have special effect depends on its value, instead, a separate removeErrors (or different name) should be introduced to handle the scenario.

Also hit this again… this is pretty terrible that its still not sorted

Ran into this today, pretty annoying functionality

From reading the thread, it looks like the core request is a software change. I’ve removed docs-related labels, so this will drop into the engineering triage queue. If they decide no software change, then we can come back to this as a doc issue.

@kapunahelewong would a change to this behavior be accepted? I suggest to make the condition for INVALID to require that the errors object contains at least one property with a truthy value.

I can make a PR if so desired.

based on @salazarr-js’ answer it might be prettier with && operator:

import _omit from 'lodash-es/omit';

control.hasError('errorToRemove') && control.setErrors(_omit(control.errors, ['errorToRemove']))

@taitgremp

to do a bunch of extra work

The error object is a simple plain object with a “key” - “value” relationship.

The extra work im talking about is having to check the object for emptiness since errors MUST be null right now in order for the control to be valid. I shouldnt have to worry about that object, i should be able to have different kinds of errors, and be able to just remove one of them, and have angular forms do the work of removing the specified error and making the control valid if there were no other errors.

I can see how the current behavior is useful. The presence of a property on the errors object indicates it has an error, and the value of that key can be set to any additional data you may want to use. It may make sense to have a falsy value there in some scenarios. Therefore, I am not sure the current behavior needs to change in this regard.

However, I think the current API does lack an efficient way to add and remove errors on controls without overriding other errors. This seems like such a common use case that having some methods (similar to what @KostaMadorsky has implemented) available would be reasonable.

Hello. I ran into this at work today. Why is there not a simple way to add or remove errors?

I don’t want to have to do a bunch of extra work just to remove one error without removing different errors, a feature request to add functions like addError() removeError() would be nice

I totally agree with this issue. AngularJS was a lot better with validators. setErrors is a mess to set as long as you have more than one error to set. You are forced to use several if tests and properly handle the case without any error.

.setError(key,value?) and .clearError(key),

This would helps, althrough not perfect either.

What about updating ValidationErrors type ? Currently: export declare type ValidationErrors = { [key: string]: any; };

Proposal: export declare type ValidationErrors = { [key: string]: boolean|object; };

boolean would be the most used case to indicate error status (valid or invalid). A boolean type wouldn’t bring any additionnal information anyway. object (or any ?) would be used to handle special case when you need to provide additionnal informations.

Consider a simple and common case with current implementation:

if (this.empty && this.required) {
   this.formControl.setErrors({
      empty: true
   });
} else if (!this.empty && !isValidJSON) {
   this.formControl.setErrors({
      jsonSyntax: true
   });
} else {
   this.formControl.setErrors(null);
}

It is really awkward and impratical. This could be changed to something more friendly:

this.formControl.setErrors({
   empty: this.empty && this.required,
   jsonSyntax: !this.empty && !isValidJSON
});

If I need more additionnal informations:

this.formControl.setErrors({
   empty: this.empty && this.required,
   jsonSyntax: !this.empty && !isValidJSON ? { error: "...", line: 5 } : false
});

I would be fine with empty object being valid input. But that isn’t the case. Now I have to take current or create empty error object, add/remove specific keys to it and at the end, I have to check for empty object and if it is, replace it with null… this is lot of code for pretty obvious problem, and still wouldn’t work with async validators. I don’t see how this is expected behavior in any scenario.

I could live with it if I could use something like .setError(key,value?) and .clearError(key), but alas, I cannot do that either. Properly producing in/valid state from multiple validators is basically impossible right now.

@manklu While it could be breaking in some scenarios, it won’t break normal/sensible ones.

Correct me if I’m wrong, but the only implementations that will be negatively effected by this change would be those that rely on the following errors values producing an INVALID state.

{}
{
    max: false
}
{
    max: null
}
{
    max: undefined
}
{
    max: 0
}
{
    max: NaN
}
{
    max: ''
}

Post-change, these errors values would not produce an INVALID state.

@jdhines thank you for bringing this up (and @krodyrobi for chiming in too!). I’ve reached out to Kara and gotten some clarification. Here’s my paraphrasing:

The validation functions take a value and return either a map of errors (INVALID) or null (VALID). The name of your error in the map can be anything while the value of the error can be any extra info; for example, the maxLength error includes the expected length and the actual length. However, other errors dont have any extra info; for example, required just has "true" ({required: true}). So even if an error value is false in the map, the error is assumed to exist. You can only indicate that an error doesn’t exist if the error is absent from the map or if the errors property is null.

Does this help? We can add this into the docs to help anyone who runs into this. Let me know if anything is still fuzzy and I’ll find the answer.