Using 'ForInStatement' is not allowed (no-restricted-syntax)

[francoisromain]

Since version 8, there is this error on for in loops.
What should be used in place of for-in loops ?
Thanks

[softwarespot]

I was literally thinking the same 20 minutes ago and couldn't see any mention of this in the README.

[ljharb]

https://github.com/airbnb/javascript#iterators--nope

Use things like map/every/some/filter/reduce/find/findIndex etc to iterate over arrays, and Object.keys/Object.values/Object.entries to produce arrays so you can iterate over objects.

Feel free to provide some loops (ideally concrete use cases and not contrived examples), and I'll happily show you a better way to write the code with iteration instead of a loop.

[softwarespot]

Of course, but maybe this should be added to the README under Objects?

[ljharb]

PRs are welcome to improve the documentation.

[francoisromain]

@ljharb thank you very much

[francoisromain]

@ljharb

my loop was like this:

for (const i in options) {
    if (opts && opts.hasOwnProperty(i) && options.hasOwnProperty(i)) {
        options[i] = opts[i];
    }
}

and it is now like this:

Object.keys(options).forEach((key, index) => {
    if (opts && opts.hasOwnProperty(key)) {
        options[key] = opts[key];
    }
});

is there a better way?

[ljharb]

@francoisromain Object.assign(options, opts)

[francoisromain]

@ljharb oh wow! : )

[francoisromain]

@ljharb it's not really the right place to ask, here is an unrelated question:

Is there a better syntax for this one?

if (value) {
  options.value = value;
}

Thank you

[ljharb]

@francoisromain no, that is the proper way to do it, so that you only perform an assignment in the case where the conditional is true.

[francoisromain]

@ljharb ok thanks

[sveip]

Another one for you @ljharb :)

let count = 0;
const selected = this.state.selected;
for (const wanted in selected) {
  if (selected.hasOwnProperty(wanted)) {
    if (selected[wanted] === true) {
      count++;
    }
  }
}

[ljharb]

@sveip

const count = Object.values(this.state.selected).filter(Boolean).length;
/* or, if you really need to only check for literal true */
const count = Object.values(this.state.selected).filter(x => x === true).length;

[jchbh-duplicate]

if use for-in/ for-of with async generator function, how to change it?

Promise.coroutine(function*() {
  const files = ['f1', 'f2', 'f3']; 
  for (let file of files) {
     yield fs.readFileAsync(file);
  }
})

By the way, this code can be replace into async-await style in ES7 syntax.

[ljharb]

I don't know what Promise.coroutine is, since that's not part of standard Promises (and JS can't truly have actual coroutines).

For the body of the generator though, Promise.all(files.map(file => fs.readFileAsync(file))) gives you a promise of the results if you want them read in "parallel". Certainly if you want generator semantics, you need for..of there, but since our styleguide discourages use of generators…

[EvanCarroll]

@sveip

Or, with _.reduce:

let countTrue = ( input, selected ) =>
    _.reduce(input, (result,v,k) => { if(selected[k] === true) {result++} return result; }, 0 );

countTrue( {a:1,b:2,c:3,d:4}, {a:true,b:true,z:true} ); // 2
countTrue( {a:1,b:2,c:3,d:4}, {a:true,b:false,z:true} ); // 1

[EvanCarroll]

There are a few instances where for...in statements are useful, here is one. If you're using generators for async code (as in the case of Co.js and Koa),

You can do this..

Object.keys(file.data.languages.data).forEach( key => {
  // stuff with access to key...
} );

That works, but now if you want to make it async this won't work.

Object.keys(file.data.languages.data).forEach( key => {
  yield stuff();
} );

You can go back to this..

for (key in file.data.languages.data) {
  if ( !file.data.languages.data.hasOwnProperty(key) ) { continue; }
  yield stuff();
}

[kesne]

It's worth noting that the styleguide doesn't allow generators, so if you're overriding that rule for you project, you could also override the for-in rule.

[EvanCarroll]

Yea, well not allowing generators was all kinds of silly being that they're the easiest path to upgrading to async/await, and babel is a pain in the rear for development.

[ljharb]

Even with generators, you never want for..in with objects. You'd do for (const key of Object.keys(obj)) { yield stuff(key, obj[key]); }

[mhofman]

So what is one supposed to do if they want to iterate over all enumerable properties of an object, not only their own properties? I don't see any solution besides a very cumbersome Object.getPrototypeOf loop and remembering which properties were already visited to avoid revisiting "overridden" properties.

The problem with the no-restricted-syntax rule is that to re-enable the for ... in restricted syntax locally, you have to re-emit the rule with all the other restricted syntax statements minus the ForInStatement, which is very error prone.

[ljharb]

@mhofman
I agree that overriding this rule is annoying, as I've had to do it in a couple modules that build abstractions around for..in.

I'm curious, what's your use case for wanting inherited properties?

In general, the thing you want is an ES spec proposal: Object.enumerable{Keys,Values,Entries}, but it doesn't exist yet. There used to be Reflect.enumerate but that's now removed.

[mhofman]

My use case is a logging library that enumerates over all properties to log them. I agree it's pretty rare to actually have to use for ... in but the lack of alternatives right now makes its syntactic disabling very annoying.
I thought the guard-for-in was a pretty good way of enticing people to avoid using it, while still allowing its use when necessary (easy rule to disable).

[ljharb]

guard-for-in just requires any conditional check - often it false-negatives on places where people aren't checking "own property", precisely to allow the use case you mean.

If eslint added a more specific way to block for-in loops or an easier way to override it, I'd be all for that - but imo the annoyance of overriding it for a small handful of edge cases is worth the benefit of never having to see a for-in loop anywhere else :-)

[EmilyRosina]

@ljharb 👋 how about...

const aliases = {
  '@': 'src',
  shoelace: path.resolve(__dirname, 'src/components/shoelace'),
}

function resolveSrc(_path) {
  return path.join(__dirname, _path)
}

function registerWebpackAliases() {
  const webpackAliases = {}
  for (const [alias, aliasPath] of Object.entries(aliases)) {
    webpackAliases[alias] = resolveSrc(aliasPath)
  }
  return webpackAliases
}

[ljharb]

@EmilyRosina

return Object.fromEntries(Object.entries(aliases).map([
  alias,
  aliasPath,
] => [
  alias,
  resolveSrc(aliasPath),
]));

(See https://npmjs.com/object.fromentries if you need to polyfill it)

[lincolnaleixo]

@ljharb Is it possible to convert a function that has async sleep?

for (const postLink of postLinks) {
  await likePost(postLink);
  await sleep(3000);
}

PS:
const sleep = milliseconds => new Promise(resolve => setTimeout(resolve, milliseconds));

[karol-majewski]

@robot110011

postLinks
  .reduce(
    async (previousPromise, post) => {
      await previousPromise;
      likePost(post);

      return sleep(3000);
    },
    Promise.resolve(),
  )

[lincolnaleixo]

Wow thanks @karol-majewski ! Not good to read tough

[lincolnaleixo]

Is this loop acceptable by the guide?

for (let index = 0; index < postLinks.length; index += 1) {
  await likePost(postLinks[index]);
  await sleep(3000);
}

[ljharb]

@robot110011 there's no "sleep" in JS; i'd name that delay. but i'd do this:

postLinks.reduce((prev, post) => prev.then(() => likePost(post)).then(() => sleep(3e3), Promise.resolve());

All loops are discouraged by this guide.

[lincolnaleixo]

Really appreciate the lesson! Cheers

[crobinson42]

Early returning in a loop is the use case for using for in and for of, ie:

for ( const task of taskList) {
  // return from loop
  if (someCondition === someTestCondition) return
  
  // else continue iterating in loop
}

[ljharb]

@crobinson42 for those use cases, it's better to use (or build) an iterating abstraction like .some or .find etc, so that your intention is clear. Often this can mean, breaking the loop body up into multiple iterations, if it's conceptually doing multiple tasks (like a map + find)

[rodrigoabb]

It might be worth to notice: there are things that can't be (easily) done with Object.keys/Object.values/Object.entries with a foreEach() as suggested. For example, if we use a filter, we will need to return true or false inside. As it's said here forEach() cannot be stopped, so we can't return/break. We could throw an exception though, but it's a really bad practice.

[ljharb]

@rodrigoabb some/every/find/findIndex short-circuit, ftr. show me some code that you think can't be easily done with iteration methods, and i'll be happy to supply you with code that is.

[joaocunha]

This thread is already 3 years old and you're still teaching and helping people out with their iterations. You're such a good sport, @ljharb.

[georgegrigorita]

I know it's an old thread, but I was wandering how to rewrite this for...in loop:

const result = products.flatMap((obj) => {
  const parts = [{}, {}];
  for (const key in obj)
    parts[key.startsWith('user') ? 1 : 0][key] = obj[key];
  return parts.filter((value) => Object.keys(value).length !== 0);
});

In this case products is an array of objects:

const products = [{
    "listPrice": 50,
    "discount": 10,
    "total": 45,
    "users": "",
    "userNumber": 10,
    "userlistPrice": 120,
    "userDiscount": 10,
    "userTotal": 108
  },
  {
    "listPrice": 1000,
    "discount": 10,
    "total": 900,
    "userNumber": 100,
    "userlistPrice": 1200,
    "userDiscount": 0,
    "userTotal": 1200
  }]

[ljharb]

@georgegrigorita

function split(array, predicate) {
  return array.reduce(([a, b], item) => {
    if (predicate(item)) {
      return [a, b.concat(item)];
    }
    return [a.concat(item), b];
  }, [[], []]);
}

const result = products.flatMap((obj) => {
  const parts = split(Object.entries(obj), ([k]) => k.startsWith('user'));
  return parts.filter((v) => v.length > 0).map(Object.fromEntries);
});

maybe something like this?

[mkovel]

@rodrigoabb some/every/find/findIndex short-circuit, ftr. show me some code that you think can't be easily done with iteration methods, and i'll be happy to supply you with code that is.

Hi @ljharb, I woudlike to know your opinion about how to rewrite this code?

The main point is that I need process chunks in series,
not in "parallel" but urls in one chunk in "parallel".

const chunks = [ ['url1', 'url2'], ['url3', 'url4']]; 

for (const chunk of chunks) {
  await Promise.all (
    chunk.map( url => fetch(url)
  )
}

[ljharb]

@mkovel

chunks.reduce(async (prev, chunk) => {
  await prev;
  return Promise.all(chunk.map(url => fetch(url)));
}, Promise.resolve());

if you need the fetched data, it'd be a little different, but your snippet doesn't save that info, so neither does mine.

[mkovel]

@ljharb Thank you for answer.
Yeap, "reduce" can help with it and saving data is not a main point.
The main point in my question was about clarity and predictability of solution.
And which variant do you think looks more clear and predictable?

In personal for me (await prev and Promise.resolve() as initial value) - looks a non-obvious
I am not about your solution the code is absolutely correct I am about composition and style in general.

Do you agree with me? )

[ljharb]

Nope. I think the harm caused by loops is far outweighed by the slightly less intuitive reduce form above.

[mkovel]

:) Hmm... interesting
Did you mean harm from loops in general or some specific?
What harm do you know in my example (performance or what)?

[ljharb]

@mkovel http://qr.ae/RNMEGA and https://gist.github.com/robotlolita/7643014 are worth a read. Happy to discuss further in the gitter channel if you have further questions, since it's related, but off topic, for this issue.

[lingtalfi]

@ljharb
Another one (if the offer is still on the table):

  for (const story of props.stories) {
    progressTracker.value[story.id] = 0;
  }

I'm curious how would one make this more concise, or at least make it eslint rule compliant.

[lingtalfi]

Ok nevermind, I found it:

  Object.keys(props.stories).forEach((key) => {
    progressTracker.value[props.stories[key].id] = 0;
  });

[EmilyRosina]

@ljharb Another one (if the offer is still on the table):

  for (const story of props.stories) {
    progressTracker.value[story.id] = 0;
  }

I'm curious how would one make this more concise, or at least make it eslint rule compliant.

props.stories.forEach(({ id }) => progressTracker.value[id] = 0)

Does this work?

[lingtalfi]

@EmilyRosina
Indeed, this works:

  props.stories.forEach(({id}) => {
    progressTracker.value[id] = 0;
  });

thanks for the hint (it's much more readable than my previous try).