sheerun / prettier-standard

Formats with Prettier and lints with ESLint+Standard! (āœæā— ā€æā— )
MIT License
868 stars 44 forks source link

discrepency between standard and prettier-standard #34

Closed dylanjha closed 6 years ago

dylanjha commented 6 years ago

Using:

My goal here, which I think is a common use case is to use prettier-standard in a pre-commit hook to format the code that gets committed. I also want to have the code that gets committed be compliant with StandardJS. Ultimately, the CI server will run the standard CLI command on the relevant directories to confirm that the code passes the StandardJS linting rules.

I have a couple examples below where the code passes the standard command before running prettier-standard but does not pass the standard command after prettier-standard does the formatting.

I have supplied a small gist here with the starting code for both Example 1 and Example 2: https://gist.github.com/dylanjha/1d07f355636e15293df46c9db6781c5d

Thank you to anyone who can chime in šŸ‘ . Perhaps I'm going about this the wrong way or missing something obvious here.

Example 1

I have a file that looks like this:

const obj = {}

function doSomething () {
  return new Promise((resolve, reject) => {
    obj.create({
      name: 'Dylan',
      username: 'dylan',
      photo: 'avatarurl.com',
      email: 'dylan@myemail.com'
    },
    { url: window.location.href })
    .then(() => {})
  })
}

doSomething()

Right now, this file is compliant with StandardJS, running the standard cli passes.

Now I run prettier-standard, here's what I get, which looks nice šŸ’…

const obj = {}

function doSomething () {
  return new Promise((resolve, reject) => {
    obj
      .create(
        {
          name: 'Dylan',
          username: 'dylan',
          photo: 'avatarurl.com',
          email: 'dylan@myemail.com'
        },
        { url: window.location.href }
      )
      .then(() => {})
  })
}

doSomething()

Problem is, this file is no longer StandardJS complaint, running the standard CLI fails:

testing/gist.js:7:9: Expected indentation of 6 spaces but found 8.
testing/gist.js:8:11: Expected indentation of 8 spaces but found 10.
testing/gist.js:9:11: Expected indentation of 8 spaces but found 10.
testing/gist.js:10:11: Expected indentation of 8 spaces but found 10.
testing/gist.js:11:11: Expected indentation of 8 spaces but found 10.
testing/gist.js:12:9: Expected indentation of 6 spaces but found 8.

Example 2

Starting with this code

function doSomething () {
  return new Promise((resolve, reject) => {
    return reject( // eslint-disable-line prefer-promise-reject-errors
      'reject this thing'
    )
  })
}
doSomething()

StandardJS has a rule to pass in error objects to reject (not strings). This rules is being explicitly disabled with the in-line comment, so this code passes the StandardJS standard CLI command.

When prettier runs, it reformats the code to look like this:

function doSomething () {
  return new Promise((resolve, reject) => {
    return reject(
      // eslint-disable-line prefer-promise-reject-errors
      'reject this thing'
    )
  })
}

doSomething()

Now, because the eslint-disable comment was moved to it's own line, the code is no-longer compliant with StandardJS and no longer passes the standard command.

sheerun commented 6 years ago

Hey!

These are desired behaviors. In the "Example 1" we prefer whitespace formatting of prettier before standard's rules. This is whole point of prettier-standard. Not sure how I can help here :/

As for the second example, for the same reason you'd need to use eslint-disable-next-line

dylanjha commented 6 years ago

@sheerun thanks.

Is it not common (nor suggested) to both use prettier-standard (for formatting) & run standard (for linting)

(from the README):

While standard is a linter, prettier-standard is a formatter.

For example, I want to use the linter to catch things like unused variables, not handling errors, etc. What is the recommended way to achieve this?

sheerun commented 6 years ago

You need to disable whitespace checking in your .eslintrc, something like so:

{
  "rules": {
     "indent": "off"
  }
}
dylanjha commented 6 years ago

Gotcha, thanks for that. One thing I'm a little unclear about:

Either way, as I'm trying this it doesn't seem to be working:

  1. add .eslintrc with {"rules": {"indent": "off"}}
  2. Using Example 1
  3. Run standard --fix (fixes whitespace so standard is happy)
  4. Run prettier-standard - whitespace gets fixed according to prettier 's rules, so now standard is no longer happy

Using: standard (10.0.3) prettier-standard (7.0.3)

I made a repo here demonstrating the above example: https://github.com/dylanjha/sample-prettier-standard, there is a .eslintrc file in the root directory. If anyone pulls this down you can run:

  1. yarn lint (standard passes)
  2. yarn format-prettier
  3. yarn lint (standard fails)

Thank you for taking the time, appreciate the help. Cheers šŸ»

sheerun commented 6 years ago

both eslint and prettier-standard will respect that .eslintrc (but prettier-standard ignores any non-fixable rules, and indentation is probably one of them).

I'm not sure that standard respects .eslintrc, you probably need to use https://github.com/standard/eslint-config-standard instead

Also prettier-standard should be run before eslint/standard

Hope you manage to fix your setup

dylanjha commented 6 years ago

Gotcha, thanks sheerun. In my example prettier-standard is fixing indentation rules, but it is ignoring the .eslintrc config. Never got to the bottom of that, but that ended up not being a blocker for me. In case anyone finds this thread, I managed fixing my setup by doing this:

Pre commit hook:

On CI: