javascript 🚀 - Disallow for-of

Absolutely, thanks! Want to make a PR? it will be semver-major, so i won't be able to merge it immediately, but having it queued up would be great.

ljharb on 13 Oct 2016

Looks like I was beat to the punch by mere hours. Thanks @SimenB.

aboyton on 17 Oct 2016

What should I do in this case:

async function foo(args) {
  for (const arg of args) {
    await bar(arg);
  }
}

dplusic on 11 Nov 2016

👍51

@dplusic in nice, simple ES6:

function foo(args) {
  return args.reduce((prev, arg) => prev.then(() => bar(arg), Promise.resolve());
}

ljharb on 11 Nov 2016

👎192 😕23 😄21 👍8 👀4 🚀2

It works. Thanks!

dplusic on 11 Nov 2016

👍2 😄1

@ljharb - what about generators?

I have a function like this that flattens objects recursively, so { user { name: "Lee", location: { city: 'London' } } } becomes {'user.name': 'Lee', 'user.location.city': 'London'}:

function* flatten(obj, str = '') {
  if (obj) {
    for (const name of Object.getOwnPropertyNames(obj)) {
      if (!Array.isArray(obj) && typeof obj[name] === 'object') {
        yield* flatten(obj[name], str ? `${str}.${name}` : name);
      } else {
        yield {
          [`${str}.${name}`]: obj[name],
        };
      }
    }
  }
}

Which is turned into a 'flat' object with Object.assign({}, ...flatten(obj));

I'm probably overlooking an obvious equivalent with map, but a generator solves this quite neatly...

leebenson on 18 Nov 2016

@leebenson generators are not allowed in our guide.

As for an alternative, I'm not sure if this actually works, but perhaps something like:

function flattenObject(obj, prefix = '') {
  return Object.entries(obj).reduce((acc, [key, val]) => {
    const prefixedKey = prefix ? `${prefix}.${key}` : key;
    if (val && !Array.isArray(val) && typeof val === 'object') {
      return Object.assign(acc, flattenObject(val, prefixedKey));
    }
    return Object.assign(acc, { [prefixedKey]: val });
  }, {});
}

ljharb on 18 Nov 2016

😄1

Thanks @ljharb. This minor tweak works perfectly:

function flattenObject(obj, prefix = '') {
  return Object.entries(obj).reduce((acc, [key, val]) => {
    const prefixedKey = prefix ? `${prefix}.${key}` : key;
    if (val && !Array.isArray(val) && typeof val === 'object') {
      return Object.assign(acc, flattenObject(val, prefixedKey));
    }
    return Object.assign(acc, { [prefixedKey]: val });
  }, {});
}

leebenson on 18 Nov 2016

🎉1 👍1

@dplusic @ljharb
I don't understand. Where's the advantage in making your code totally unreadable?
Purity is nice, but not when it takes 30 minutes to decipher a simple function.

minexew on 5 Dec 2016

👍114 🎉8 😄7 ❤6

Of course readability is subjective, but I find loops far more unreadable than a functional alternative.

ljharb on 5 Dec 2016

👎24 😕2 👍1

That's interesting. Of course I don't have any empirical evidence for this, but I bet it'd take the average programer an order of magnitude more time to understand your snippet vs what @dplusic posted.

I hope this fad of functional-at-all-cost is over soon. There's a reason why Haskell hasn't gone mainstream.

minexew on 5 Dec 2016

👍66 👎2

I do think @minexew has a point.

With native async/await landing in V8, I think it's going to be pretty common pattern to 'unwind' functional logic like this. for...of and generators often make the code more readable, and easier to follow. I'm not sure effectively banning swaths of valid JS is the way to go.

Personal preference, of course.

leebenson on 5 Dec 2016

👍52

i’d actually argue that for..of should be used because of functional style:

.forEach() isn’t functional. it exists for its side effects. so if anything, .forEach() should be banned and for..of allowed in order to make clear that side effects are happening here (which isn’t as apparent from a filter → map → forEach pipeline)

flying-sheep on 6 Dec 2016

👍57 👎1

@flying-sheep it's a functional evolutionary step to map and friends. for..of does not lend itself towards evolving to be a proper map, filter, etc. In addition, .forEach forces side effects (or else nothing really happens) - for..of can also be used not for side effects.

ljharb on 6 Dec 2016

👎8 👍1

behold:

function* map(callable) {
    for (const element of this) {
        yield callable(element)
    }
}
function* filter(predicate) {
    for (const element of this) {
        if (predicate(element)) yield element
    }
}

const odd = '12345'
    ::map(parseFloat)
    ::filter(e => e % 2)

In addition, .forEach forces side effects (or else nothing really happens) - for..of can also be used not for side effects.

both only have side effects (if anything, .forEach returns undefined). what do you mean with “not for side effects”

flying-sheep on 7 Dec 2016

@flying-sheep lol fair point, both are for side effects since for..of isn't an expression. forget that part.

The :: operator is stage 1, and Airbnb only recommends using stage 3 proposals and above. Similarly, transpiling generators requires regenerator-runtime, which is way too heavyweight to ship to production, so our config also forbids generators at this time.

ljharb on 7 Dec 2016

Similarly, transpiling generators requires regenerator-runtime, which is way too heavyweight to ship to production, so our config also forbids generators at this time.

Kinda tangentially related: would you be interested in a config for server-side only projects? I've used airbnb-base at it works fairly fine, but e.g. trailing commas in functions has to be disabled, and generators could be allowed, etc. There's not a lot of rules that needs tweaking, but maybe you could also include https://github.com/mysticatea/eslint-plugin-node.

If you're interested at all, I can create a new issue for it to discuss its merits there? 😄

SimenB on 7 Dec 2016

Server-side only projects should be using babel also.

ljharb on 7 Dec 2016

😕20 👎5

Why? With async/await landing, the only syntax I care about is object spread, and it's OK to use Object.assign. Waiting for trailing function commas until support lands isn't an issue either. Adding the overhead of a build step seems unnecessary.

SimenB on 7 Dec 2016

👍1

For one, old syntax is generally faster than new syntax, since it's had time to be optimized. Additionally, there'll always be new syntax you want to use, and if your build process is already in place, it's trivial to add that.

A build step shouldn't add overhead, since you should be running your linter and your tests prior to code going to production anyways - just add the build step into there.

ljharb on 7 Dec 2016

👎3 👍2

@ljharb maybe i didn’t understand you: you argued that it’s an evolutionary step towards functional array methods. i showed that this isn’t the case. my use of generators is irrelevant. e.g. here’s a map implementation without anything you mentioned:

```js
Array.prototype.map = function map(callback) {
const mapped = []
for (const element of this) {
mapped.push(callback(element))
}
return mapped
}

flying-sheep on 7 Dec 2016

The given implementation of map - or any other abstraction - is entirely irrelevant. forEach is an array prototype method, just like the functional array prototype methods.

ljharb on 7 Dec 2016

I use my babel-preset-node7 for server side use of the airbnb preset. It adds the (very few) missing pieces -- trailing function commas, etc -- and avoids transpiling native features like generators, async/await, etc.

The extra build step makes sense in my case, because I'm writing universal code that targets both the server and the browser, so a build step is necessary anyway. I use generators, async/await, etc freely, and provide a different babel config to each target in webpack. In my use case, adding browser polyfills/runtimes has a negligible effect on the bundle size. My total app is less than 200kb gzipped, with React, Mobx, es2015 polyfills, all app code, etc.

For the most part, I find the airbnb preset to match my own personal stylistic preferences, but I do agree that eliminating for...of is a step too far. I find generators to be very useful for lazy evaluation and for..of (especially coupled with generators and await) makes the code path very easy for others in my team to follow.

In my case, adding a custom rule for no-restricted-syntax fixes my one irritation.

leebenson on 7 Dec 2016

yeah, for..of and generators can be used e.g. for efficient parsers (you delegate to sub parsers via yield*). some languages run on those features extensively, e.g. python and rust.

JS is always the slow kid on the block. maybe in a year or so, we’ll see “map considered harmful” posts that praise how much awesomer generators and comprehensions are.

flying-sheep on 7 Dec 2016

Comprehensions are never coming to JS, so I doubt you'll ever see that post :-)

ljharb on 7 Dec 2016

Let's see 😁