Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eslint + prettier produces non-viable code #1987

Closed
maloguertin opened this issue Jan 10, 2019 · 10 comments
Closed

eslint + prettier produces non-viable code #1987

maloguertin opened this issue Jan 10, 2019 · 10 comments
Labels

Comments

@maloguertin
Copy link

maloguertin commented Jan 10, 2019

when extending airbnb's eslint plugin along with prettier eslint and running it on this code:

const foo = (paramA, paramB, paramC) => {
  return Object.keys(paramB.prop.nestedProp[0])
    .filter(attribute => paramC.includes(attribute))
    .map(attribute => {
      return Object.keys(paramB.prop.nestedProp[0][attribute]).map(
        attributeKey => {
          return {
            key: `${paramA.name} ${paramB.type} ${attribute} ${attributeKey}`,
            value: paramB.prop.nestedProp[0][attribute][attributeKey],
          }
        }
      )
    })
}

which is ugly but working

I obtain this code:

const foo = (paramA, paramB, paramC) =>
  Object.keys(paramB.prop.nestedProp[0])
    .filter(attribute => paramC.includes(attribute))
    .map(attribute =>
      Object.keys(paramB.prop.nestedProp[0][attribute]).map(attributeKey => ({
            key: `${paramA.name} ${paramB.type} ${attribute} ${attributeKey}`,
            value: paramB.prop.nestedProp[0][attribute][attributeKey],
          })
    )

which is missing a ) at the end.

my eslint config is this

{
  "parser": "babel-eslint",
  "plugins": ["prettier", "react", "jest"],
  "env": {
    "browser": true,
    "node": true,
    "jasmine": true,
    "jest/globals": true
  },
  "parserOptions": {
    "ecmaVersion": 2017,
    "sourceType": "module",
    "ecmaFeatures": {
      "jsx": true
    }
  },
  "settings": {
    "import/resolver": {
      "webpack": {
        "config": "webpack.common.js"
      }
    }
  },
  "rules": {},
  "extends": ["airbnb", "plugin:prettier/recommended"]
}

Now this may have nothing to do with airbnb (if so feel free to close) but I thought this would be of interest to you.

@ljharb
Copy link
Collaborator

ljharb commented Jan 10, 2019

Which paren is it missing? The "after" example looks correct to me.

@maloguertin
Copy link
Author

maloguertin commented Jan 11, 2019

http://jsfiddle.net/R6DWN/45/

missing the ) to close the first map after the filter!

@ljharb
Copy link
Collaborator

ljharb commented Jan 11, 2019

ahh i see it. Can you reproduce the issue without prettier?

@maloguertin
Copy link
Author

Okay when running simply eslint with --fix and removing the "plugin:prettier/recommended" from the plugins it produces this code:

const foo = (paramA, paramB, paramC) => Object.keys(paramB.prop.nestedProp[0])
  .filter(attribute => paramC.includes(attribute))
  .map(attribute => Object.keys(paramB.prop.nestedProp[0][attribute]).map(
    attributeKey => ({
      key: `${paramA.name} ${paramB.type} ${attribute} ${attributeKey}`,
      value: paramB.prop.nestedProp[0][attribute][attributeKey],
    }),
  ));

export default foo;

which is valid so the conflict seems to be with the prettier/recommended plugin!

@ljharb
Copy link
Collaborator

ljharb commented Jan 14, 2019

Please file an issue on prettier for that, and link it here :-)

@ljharb ljharb closed this as completed Jan 14, 2019
@ljharb ljharb added the invalid label Jan 14, 2019
@sharmilajesupaul
Copy link
Contributor

sharmilajesupaul commented Jan 14, 2019

@maloguertin and other's who run into this problem, it's a conflict between our arrow-body-style rule configuration and Prettier.

So a workaround is adding "arrow-body-style": "off" and letting Prettier handle the formatting instead.

It's also worth noting that the Prettier output and the arrow-body-style autofix result in correct code when run individually, and the problem occurs when they're used together in a single config.

@maloguertin
Copy link
Author

@sharmilajesupaul thanks I'll try that!
What I'm confused about is that https://github.com/prettier/eslint-config-prettier is supposed to:

Turns off all rules that are unnecessary or might conflict with Prettier.

@brodybits
Copy link

So a workaround is adding "arrow-body-style": "off" and letting Prettier handle the formatting instead.

Shouldn't that be part of prettier/eslint-config-prettier?

@sharmilajesupaul
Copy link
Contributor

@brodybits @maloguertin yup, my guess is there's some configuration of the rule that does work with Prettier and the config we use in the style guide doesn't. I made an issue on prettier/eslint-config-prettier: prettier/eslint-config-prettier#71

@lydell
Copy link

lydell commented Jan 14, 2019

I think this is another manifestation of prettier/eslint-plugin-prettier#65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants