RFC: Prettier

[justingreenberg]

i'm surprised nobody has brought this up yet... it's been a game changer for my team. would be nice to integrate with #1711 husky pre-commit hooks. imo this is a no-brainer, kinda wish it shipped with v3.5. maybe we can tighten up the releases and cut another minor in the next couple of weeks

[mxstbr]

YES

[mihir0x69]

Wow! How could we miss this! Awesome.

[dannytce]

Prettier is definitely the must. I will be more than happy to help with that. But probably let's discuss configuration first.

I am used to using following config:

{
  "trailingComma": "es5",
  "singleQuote": true,
  "semi": false
}

As you see, I am not a big fan of semicolons. I would rather removed them from ESLint config as well. So what do you think? Semicolons yes (👍 ) or no (👎 )?

[justingreenberg]

@dannytce i share your preferences, however it's so easy to expose configuration, let's use defaults (with semi) but leave options in userland so everyone can be happy

[blling]

Hi,
Great idea but why it sinked?

[gretzky]

Would definitely love to see this happen. I'd be willing to open a PR if nobody else has yet.

[sourcesoft]

I'm currently using react-boilerplate with prettier in 2 projects, here's my config. It looks like the current eslint configuration and it also adds the prettier to precommit and removes eslint from precommit.

package.json

    "lint:staged": "lint-staged",
    "prettier": "prettier --write '**/*.js'",
...
  "prettier": {
    "trailingComma": "es5",
    "semi": true,
    "singleQuote": true
  },
  "lint-staged": {
    "**/*.js": [
      "prettier --write",
      "git add"
    ]
  },
...
    "eslint-config-prettier": "^2.3.0",
    "eslint-plugin-prettier": "^2.1.2",
    "prettier": "^1.8.2",

.eslintrc.js

module.exports = {
  parser: 'babel-eslint',
  extends: 'airbnb',
  env: {
    browser: true,
    node: true,
    jest: true,
    es6: true,
  },
  plugins: ['redux-saga', 'react', 'prettier', 'jsx-a11y'],
  parserOptions: {
    ecmaVersion: 6,
    sourceType: 'module',
    ecmaFeatures: {
      jsx: true,
    },
  },
  rules: {
    'prettier/prettier': [
      'error',
      {
        singleQuote: true,
        trailingComma: 'es5',
      },
    ],
    'arrow-parens': ['error', 'as-needed'],
    'arrow-body-style': [2, 'as-needed'],
    'comma-dangle': [2, 'always-multiline'],
    'import/imports-first': 0,
    'import/newline-after-import': 0,
    'import/no-dynamic-require': 0,
    'import/no-extraneous-dependencies': 0,
    'import/no-named-as-default': 0,
    'import/no-unresolved': 2,
    'import/prefer-default-export': 0,
    indent: [
      2,
      2,
      {
        SwitchCase: 1,
      },
    ],
    'space-before-function-paren': 0,
    'jsx-a11y/aria-props': 2,
    'jsx-a11y/heading-has-content': 0,
    'jsx-a11y/href-no-hash': 2,
    'jsx-a11y/label-has-for': 2,
    'jsx-a11y/mouse-events-have-key-events': 2,
    'jsx-a11y/role-has-required-aria-props': 2,
    'jsx-a11y/role-supports-aria-props': 2,
    'max-len': 0,
    'newline-per-chained-call': 0,
    'no-confusing-arrow': 0,
    'no-console': 1,
    'no-use-before-define': 0,
    'prefer-template': 2,
    'class-methods-use-this': 0,
    'react/forbid-prop-types': 0,
    'react/jsx-first-prop-new-line': [2, 'multiline'],
    'react/jsx-filename-extension': 0,
    'react/prefer-stateless-function': 0,
    'react/jsx-no-target-blank': 0,
    'react/require-extension': 0,
    'react/self-closing-comp': 0,
    'redux-saga/no-yield-in-race': 2,
    'redux-saga/yield-effects': 2,
    'require-yield': 0,
    'import/no-webpack-loader-syntax': 0,
  },
  settings: {
    'import/resolver': {
      webpack: {
        config: './internals/webpack/webpack.prod.babel.js',
      },
    },
  },
}

This might help if someone is going to make a PR

[shsunmoonlee]

Prettier setting above: I just tested it and works like charm! thanks @sourcesoft :)

I have one question. Is there anything that we have to be cautious when running prettier on every single js file ? For example, create-react-app prettier integration only read js file in src folder. Just curious about the reason behind this code.

"lint-staged": {
    "**/*.js": [
      "prettier --write",
      "git add"
    ]
  },

[sourcesoft]

@SeunghunSunmoonLee Sometimes we have server, configuration or temp files that need to be formatted differently compared to rest of the files. In my case I once had a /db folder with knex auto generated seed files that doesn't make sense to run lint against since it creates extra unneeded git diffs. Prettier also formats .md and .json files if configured that could be avoided if needed.

[gretzky]

Thanks @sourcesoft! I'm gonna close this for now bc I believe this is being tossed around in PRs and will def be integrated, but anyone is welcome to add to this convo

[shsunmoonlee]

@sourcesoft I just found an issue. After running 'git commit' followed by prettier, some of my node_modules were deleted automatically, started getting http://localhost:3000/__webpack_hmr net::ERR_INCOMPLETE_CHUNKED_ENCODING error messages.
I found some issues related to npm error and issue with precommit running uninstall script.
#1840
#1814
Have you faced similar issues @sourcesoft ?

[sourcesoft]

@SeunghunSunmoonLee No I haven't, it feels like a firewall problem to me. See #2083 for better implementation, mine's kind of dirty.

[blling]

Done in #2083, ready for review.

[justingreenberg]

@gretzky reopening to track PR

[gretzky]

@justingreenberg wasn't this moved to #2083 ?

[blling]

@gretzky I think we should close this after #2083 merged :), could you take some time to review #2083?

[justingreenberg]

@gretzky a nice feature of github is that by referencing this issue in the PR description using "resolves" keyword (eg "Adds prettier, resolves #2083") we can keep the issue open for tracking and then let github automatically close the issue when we merge the PR :)

[gretzky]

@justingreenberg i'm aware, sorry, i thought that PR had been merged already

[csantiago132]

@gretzky @mxstbr im using this with onchange and it also helps a lot! prettier kicks in and formats it as I save each file, so when its time to commit, all files have been processed by prettier already.

steps assuming you have prettier installed:

npm install onchange --save-dev

then I made a script in package.json

"prettier-watch": "onchange 'app/**/*.js' 'app/**/*.test.js' 'app/**/*.scss' 'stories/**/*.stories.js' -- prettier --write {{changed}}"

to run

npm run prettier-watch

so this script will keep running until you want it to. @blling I saw you did a PR for it a few months ago so you might find this interesting.

[julienben]

Anybody here interested in reviewing the Prettier integration which is supposed to make it into v3.6? Please take a look at #2244.

It's integrated with ESLint via the relevant plugin+baseConfig, calibrated for ES5 (because of Node 6 compatibility) and triggered for JS and JSON by pre-commit hooks. It could do with extra docs though.

Either way, I'll be closing this as v3.6 is getting close to release.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.