tslint / codelyzer / ng lint error: "for (... in ...) statements must be filtered with an if statement"

Question

Lint error message:

src/app/detail/edit/edit.component.ts[111, 5]: for (... in ...) statements must be filtered with an if statement

Code snippet (It is a working code. It is also available at angular.io form validation section):

for (const field in this.formErrors) {
      // clear previous error message (if any)
      this.formErrors[field] = '';
      const control = form.get(field);

      if (control && control.dirty && !control.valid) {
        const messages = this.validationMessages[field];
        for (const key in control.errors) {
          this.formErrors[field] += messages[key] + ' ';
        }
      }
    }

Any idea how to fix this lint error?

Funicle

Maybe accept an answer? – Mogerly

Answer 1

To explain the actual problem that tslint is pointing out, a quote from the JavaScript documentation of the for...in statement:

The loop will iterate over all enumerable properties of the object itself and those the object inherits from its constructor's prototype (properties closer to the object in the prototype chain override prototypes' properties).

So, basically this means you'll get properties you might not expect to get (from the object's prototype chain).

To solve this we need to iterate only over the objects own properties. We can do this in two different ways (as suggested by @Maxxx and @Qwertiy).

First solution

for (const field of Object.keys(this.formErrors)) {
    ...
}

Here we utilize the Object.Keys() method which returns an array of a given object's own enumerable properties, in the same order as that provided by a for...in loop (the difference being that a for-in loop enumerates properties in the prototype chain as well).

Second solution

for (var field in this.formErrors) {
    if (this.formErrors.hasOwnProperty(field)) {
        ...
    }
}

In this solution we iterate all of the object's properties including those in it's prototype chain but use the Object.prototype.hasOwnProperty() method, which returns a boolean indicating whether the object has the specified property as own (not inherited) property, to filter the inherited properties out.

Chengteh

I'd like to notice that Object.keys is ES5. The only thing from ES6 there is for-of loop. We can iterate array in usual loop from 0 to its length and it would be ES5. – Mogerly

once more notice: if somehow this.formErrors is null, for...in just do nothing, while for ... of Object.keys() would throw error. – Thalassa

i am following the second solution but still i see the lint message. Disabled lint for time being. – Ferrule

Why don't you recommend Object.keys(obj).forEach( key => {...}) ? – Cloudy

@BenCarp - it has serious problems when running in an async function, therefore, at least to me, for ... in/of ... are considered more superior. See: #37577185 – Centring

note that avoiding null prototypes may be needed in case of using hasOwnProperty prototype method: Object.prototype.hasOwnProperty.call(object, key) – Peppard

The two solutions will behave differently if the object has numeric keys. Object.keys will numerically sort them, while the plain for in loop will maintain insertion order. Just something to watch out for as it's a bit unexpected. – Baronetage

Answer 2

A neater way of applying @Helzgate's reply is possibly to replace your 'for .. in' with

for (const field of Object.keys(this.formErrors)) {

Chiropodist

This should be the accepted answer as not only it solves the problem, it also reduces the amount of boilerplate code compared to additional conditionals such as if (this.formErrors.hasOwnProperty(field)). – Clarey

Be careful with the answer, it might break your codes. Test after you "fix" it. – Magma

This doesn't actually remove the tslint error for me. – Glissade

@HammerN'Songs check that you changed to for of instead of for in – Freiman

same problem here. error is not removed after using this – Conversationalist

remember to change the other line as well to : for (const key of Object.keys(control.errors)) – Arabele

Answer 3

for (const field in this.formErrors) {
  if (this.formErrors.hasOwnProperty(field)) {

for (const key in control.errors) {
  if (control.errors.hasOwnProperty(key)) {

Mogerly

Answer 4

use Object.keys:

Object.keys(this.formErrors).map(key => {
  this.formErrors[key] = '';
  const control = form.get(key);

  if(control && control.dirty && !control.valid) {
    const messages = this.validationMessages[key];
    Object.keys(control.errors).map(key2 => {
      this.formErrors[key] += messages[key2] + ' ';
    });
  }
});

Cisneros

Answer 5

If the behavior of for(... in ...) is acceptable/necessary for your purposes, you can tell tslint to allow it.

in tslint.json, add this to the "rules" section.

"forin": false

Otherwise, @Maxxx has the right idea with

for (const field of Object.keys(this.formErrors)) {

Cursor

Editing tslint.json....quick, easy and works great. Excellent answer! – Mangle

Answer 6

I think this message is not about avoiding to use switch. Instead it wants you to check for hasOwnProperty. The background can be read here: https://mcmap.net/q/36473/-iterate-through-object-properties

Targum