reactioncommerce / reaction-eslint-config

Reaction Eslint Configuration
MIT License
5 stars 6 forks source link

refactor: remove consistent-return rule #16

Closed kieckhafer closed 5 years ago

kieckhafer commented 5 years ago

The consistent-return rule is causing some confusion as to how it is implemented, and doesn't seem to be enforcing what we want it to enforce.

For example, take this function:

getCurrentTag() {
    if (this.Router.getRouteName() === "tag") {
      return this.Router.current().params.slug;
    }
  }

Even with the option ` enabled, this throws an error. When we try and returnreturnorreturn undefined, which is what we need this function to return, the error is still thrown.

It seems the rule requires a same-type return, for example a null or string to be returned, instead of undefined.

Because of this confusion, combined with this rule not being one of the eslint:recommended rules (see here), we are going to remove this rule.

kieckhafer commented 5 years ago

Closing in favor of #17

kieckhafer commented 5 years ago

After some discussion, seems we'd like to keep this rule. Closing.

spencern commented 5 years ago

@kieckhafer where was the discussion? Can you summarize?

kieckhafer commented 5 years ago

Basically we (collectively) were interpreting the incorrectly and thought it was doing the wrong thing, but in the end, it was doing the right thing and we misunderstood what it was supposed to be doing.