Shopify / javascript

The home for all things JavaScript at Shopify.
MIT License
251 stars 38 forks source link

Remove unnecessary IIFEs from non-test code #113

Closed GoodForOneFare closed 8 years ago

GoodForOneFare commented 8 years ago

There's a few IIFEs that don't seem to serve any purpose, e.g., in finances-report.js:

_getColumnIndexByKey(result, key) {
  if (!key) {
    return;
  }

  return (() => {
    for (let [index, each] of result.columns.entries()) {
      const column = result.columns[index];

      if (column.field === key) {
        return index;
      }
    }
  })();
}

A non-exhaustive list of IIFEs follow. Note that some of these may be necessary; watch out for switches or loops with return values:

GoodForOneFare commented 8 years ago

shopify-codemod/transform/removeUselessReturnFromTest contains an isUselessIIFE method that may be useful for this.

GoodForOneFare commented 8 years ago

Possibly fixed by mainline decaf merge. Please don't start work on this until we've finished evaluation.

lemonmade commented 8 years ago

Did decaf merge fix this @GoodForOneFare?

GoodForOneFare commented 8 years ago

@lemonmade It definitely helped, and there's a couple that I can fix with a trivial change to decaf. Once I've done that, I'll update this issue with anything that remains.

GoodForOneFare commented 8 years ago

My fix for try/catch IIFEs was accepted into mainline decaf. Doing the same for for loops turned out to be non-trivial. The number of IIFEs has greatly decreased, and I think that's good enough for now.