-
-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
New: Add 'recommended' configuration #73
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! This generally looks good to me, I just have a few documentation nitpicks.
## Options | ||
|
||
> Note: While it is possible to pass options to Prettier via your ESLint configuration file, it is not recommended because editor extensions such as `prettier-atom` and `prettier-vscode` **will** read [`.prettierrc`](https://prettier.io/docs/en/configuration.html), but **won't** read settings from ESLint, which can lead to an inconsistent experience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a sidenote: I was under the impression that people using eslint-plugin-prettier
would just use editor integrations for eslint
, because it wouldn't be necessary to use integrations for prettier
. Is it common for people to use prettier-atom
along with eslint-plugin-prettier
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally use prettier-vscode
with eslint-plugin-prettier
. I don't use use the vscode-eslint
plugin's format on save capabilities as Prettier covers a wider range of languages, and I find it a bit faster.
README.md
Outdated
{ | ||
"extends": [ | ||
"plugin:prettier/recommended" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it also necessary to use "plugins": ["prettier"]
in this config file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, configurations can turn on plugins, including their own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I had been under the impression that "plugins": ["prettier"]
was needed to use plugin:prettier/recommended
at all, but after trying it out it seems like you're correct.
Separately, maybe it would be a good idea to update ESLint's plugin-loading so that extends: plugin:foo/bar
also implies plugins: ["foo"]
, so that it wouldn't be necessary to do something like plugins: ["prettier"]
in a shareable config within eslint-plugin-prettier
itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was surprised that it works the way it does. Not sure if there's a use case for a plugin exporting a configuration without exposing its rules.
README.md
Outdated
This plugin works best if you disable all other ESLint rules relating to code formatting, and only enable rules that detect patterns in the AST. (If another active ESLint rule disagrees with `prettier` about how code should be formatted, it will be impossible to avoid lint errors.) You can use [eslint-config-prettier](https://github.com/prettier/eslint-config-prettier) to disable all formatting-related ESLint rules. If your desired formatting does not match the `prettier` output, you should use a different tool such as [prettier-eslint](https://github.com/prettier/prettier-eslint) instead. | ||
## Note | ||
|
||
If your desired formatting does not match the `prettier` output, you should use a different tool such as [prettier-eslint](https://github.com/prettier/prettier-eslint) instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: I think it would be better to keep this note next to the comments about eslint-config-prettier
. Can you move it to the Recommended Configuration
section (maybe under the first paragraph)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -33,7 +33,8 @@ error: Delete `;` (prettier/prettier) at pkg/commons-atom/ActiveEditorRegistry.j | |||
## Installation | |||
|
|||
```sh | |||
npm install --save-dev prettier eslint-plugin-prettier | |||
npm install --save-dev eslint-plugin-prettier | |||
npm install --save-dev --save-exact prettier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exact installation is recommended for prettier
https://prettier.io/docs/en/install.html
Thanks! |
Closes #72