Add prettier-eslint integration

[kentcdodds]

The atom plugin has added this (maybe @robwise can give some tips). This way you and @RobinMalfait (who created prettier-eslint-vscode) could join forces to make this package even better!

[RobinMalfait]

Maybe it is an idea to join forces and also move prettier-vscode+prettier-eslint-vscode to the prettier namespace on GitHub?

[CiGit]

I've closed #14 lately.
What's the advantage over running text -> prettier -> text -> eslint --fix -> text ?

This is already possible with 2 extensions (this one and Eslint) while saving:

{
  "editor.formatOnSave": true,
  "eslint.autoFixOnSave": true
}

Maybe luckily because the formatter runs before onsave events

It's also possible to setup a keybinding for those steps (not out of the box for now) microsoft/vscode#871

Moving to the prettier org is tracked in #41 and prettier/prettier#781

@RobinMalfait won't mind having more contributors ;-)

[kentcdodds]

I don't know how things are handled in VSCode, but I know in Atom if you tried to use Prettier and the ESLint plugin together you'd get a pretty bad experience:

Also, I'm certain that it doesn't perform nearly as well.

[CiGit]

In code you also have those blinking change due to the intermediate changes being printed.
That's the only bad thing I noticed. It may be a vscode bug/improvement.

I've seen something really nice in your plugin: you infer prettier's settings from eslint's one avoiding duplication and having config in your workspace. 👍

We will have to think about how we handle settings precedence in our case.

[kentcdodds]

Perhaps @robwise can give some tips in that regard 👍

Thanks for taking this under consideration @CiGit :)

[robwise]

Let me know if you have a speciifc question about what we're doing in prettier-atom. Currently, users can either choose to enable the eslint integration (i.e., use prettier-eslint under the hood) or not (i.e., use regular prettier). In the former case, we basically ignore every setting you've set for prettier because we let prettier-eslint pick it up entirely from the eslint configuration.

[CiGit]

@robwise Thanks for the info. What's happening if there is no configuration available for eslint?
IMHO it should then fallback to prettier's provided settings but it seems it's just the opposite of what @kentcdodds' prettier-eslint is doing.

[kentcdodds]

it seems it's just the opposite of what @kentcdodds' prettier-eslint is doing.

🤔 I'm not sure how you're getting that. It clearly states:

Defaults if you have all of the relevant ESLint rules disabled (or have ESLint disabled entirely via /* eslint-disable */ then prettier options will fall back to the prettier defaults

[CiGit]

Sorry for the confusion.
I was meaning user's provided prettier configuration which in prettier-atom is dropped.

1. Use eslint as source of truth
2. Fallback to user's config for missing settings
3. Fallback to prettier's default for the rest.

[kentcdodds]

Ah, gotcha. Yeah, with prettier-atom, we try to make it very simple: If you check the eslint integration button, then none of your other settings matter. That makes things pretty clear. But I suppose it could also make sense if you wanted to fallback to settings in the IDE if eslint is not configured...

[robwise]

@kentcdodds in fact, some users have requested this. I think it would be okay to do it, although yes, it would be a bit more complicated

[kentcdodds]

Come to think of it, now that prettier supports no semicolons, I would also like this! 😅

[robwise]

@kentcdodds If we somehow allow passing the fallback prettier options to prettier-eslint when invoking it, then I can hook this up pretty easily in prettier-atom.

[kentcdodds]

Yep, that's possible 😀 https://github.com/prettier/prettier-eslint/blob/master/README.md#prettieroptions-object

[robwise]

I guess we'd do the same thing but call it fallbackPrettierOptions or something?

[CiGit]

Great! prettier/prettier-eslint#73 👍

Question to prettier-eslint users:
Does it make sens to use it without having it as a project dependency ? $HOME/.eslintrc
If not we could simply pick it up from the project's dependencies and use it, thus avoiding an option.

[kentcdodds]

I could have a project with eslint configured, but not prettier-eslint. I would prefer to have it be: default to locally installed version, if it doesn't exist, use the plugin's version. Does that make sense?

[CiGit]

Definitely. So I won't remove the option

Does it make sens to load prettier-eslint from the project? If I understand it correctly prettier-eslint will load eslint/prettier from the project if I give it a filePath
But do you expect some output changes between prettier-eslint's different versions? Therefor project's version may differ from bundled one?

[kentcdodds]

Yeah, I don't expect the output too differ. It's probably fine to just use the plugin's version.

[CiGit]

Thanks for your time.
Will update my PR accordingly and wait for a review (there are a lot of changes involved)

By the way, nice work on your eslint-prettier, I've played around with it in my vscode integration and it's well better than prettier -> eslint --fix 😉

[kentcdodds]

Wonderful! Debugging tip: open up Dev tools and set: process.env.LOG_LEVEL = 'trace'
Then run the format command and you'll get a bunch of useful logs from prettier-eslint 😀

[CiGit]

a bunch ....
Thanks for the tip. It also made me correct my definitions 😆

diff --git a/src/types.d.ts b/src/types.d.ts
index f8ef5b9..d0a4ab0 100644
--- a/src/types.d.ts
+++ b/src/types.d.ts
@@ -71,7 +71,7 @@ interface PrettierEslintOptions {
     /**
      * The level for the logs
      */
-    loglevel?: LogLevel;
+    logLevel?: LogLevel;
     /**
      * Run Prettier Last. Default false
      */

[kentcdodds]

Thanks so much for doing this. It's great to converge solutions :)

[github-actions]

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