-
-
Notifications
You must be signed in to change notification settings - Fork 427
fix(prettier): use recommended rules to avoid eslint conflicts, revert #797 #827
fix(prettier): use recommended rules to avoid eslint conflicts, revert #797 #827
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.
Is this related to #826 ?
@clarkdo nah it's an unrelated issue.
|
30da4de
to
3ea2aa0
Compare
I just noticed that this PR reverts #797, where it's suggested that this is "generally not recommended by the Prettier official". However, even though it may not be recommended when using Prettier as a standalone tool, the prettier/eslint-config-prettier: "Turns off all rules that are unnecessary or might conflict with Prettier." |
3ea2aa0
to
5f05d22
Compare
Updated version to 3.4.1 (though I can't find this version on https://github.com/prettier/eslint-plugin-prettier/releases/) and resolved conflicts. By the way, should we also resolve conflicts if they are caused only by snapshot tests? |
I apologize for any disturbance caused by the PR I created. {
"editor.defaultFormatter": "esbenp.prettier-vscode", // insert this line
"editor.formatOnSave": true, // optional
} This is the quote from official prettier.
Nuxt.js does recommends VS Code as an editor, and VS Code supports Prettier. If you want to use the script in packages.json, I will create a PR with the script adjusted like this. {
"name": "eslint-prettier-test",
"version": "1.0.0",
"private": true,
"scripts": {
"dev": "nuxt",
"build": "nuxt build",
"start": "nuxt start",
"generate": "nuxt generate",
"lint:js": "eslint --ext \".js,.vue\" --ignore-path .gitignore .",
"prettier:js": "prettier --ignore-path .gitignore **.js **.vue", // insert this line
"lint": "yarn prettier:js -c; yarn lint:js", // change this line
"lint:fix": "yarn prettier:js -w; yarn lint:js --fix" // insert this line
},
"dependencies": {
"core-js": "^3.15.1",
"nuxt": "^2.15.7"
},
"devDependencies": {
"@babel/eslint-parser": "^7.14.7",
"@nuxtjs/eslint-config": "^6.0.1",
"@nuxtjs/eslint-module": "^3.0.2",
"eslint": "^7.29.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-nuxt": "^2.0.0",
"eslint-plugin-vue": "^7.12.1",
"prettier": "^2.3.2"
}
}
|
Thanks for the clarification, we now have the context behind the decision: We've removed Prettier from ESLint which was previously able to apply Prettier directly, even as part of its own It seems you're right and this issue of eslint conflict can be resolved by just running prettier as part of lintfix before running eslint. I already have an open PR #829 where I suggest adding a
Back to the topic of ESLint conflict, even though I'd prefer running only eslint on the generated code (like JSON schema parsers) without having to also run prettier (causing unnecessary code diff), it seems the same A note about your suggested {
"files.eol": "\n",
"editor.rulers": [80],
"editor.codeActionsOnSave": ["source.fixAll"],
"editor.defaultFormatter": "esbenp.prettier-vscode",
"[vue]": {
"editor.defaultFormatter": "octref.vetur"
}
}
(And of course, I don't agree that VSCode is the only de-facto IDE, as people often prefer superior products like IntelliJ IDEA rather than eating up all Microsoft's bad decisions. However, in case of Vue development I noticed IDEA having quite a lot of issues at the moment, including the inability to recognize & jump to components, reporting false-positive errors in templates, and also being unable to configure Prettier as a default formatter, having to be used manually with a different key-bind) Based on nuxt/nuxt#9563 there seems to be a trend of preferring the usage of prettier via ESLint, so we should consider which default to go forward with. Some points to consider:
|
Thanks for the reply. About generated js. If you don't mind, can you tell me the procedure for generating the problematic js file? I don't know if I can help you, but I'll look into it. I didn't know editor.formatOnSave setting is not working when autoSave is enable. As such, In some cases we need lintfix script. I don't agree that VSCode is the only de-facto IDE me too. But installing eslint-plugin-prettier does not solve the IDE problem. eslint-plugin-prettier solves this problem by running prettier during "eslint --fix", but lintfix script is the solution for editors that cannot run eslint. This description by Prettier is also important.
This is a trivial issue for those of us who are somewhat familiar with it. However, for beginners, it is one of the factors that make the hurdle higher. I often teach beginners how to develop with Nuxt.js and other development methods, and they struggle because they can't decide if the problem provided by eslint-plugin-prettier is a syntax problem or just a formatting problem. |
About the
|
@scscgit that's why I posted a how-to on stackoverflow to provide a straightforward configuration for newcomers. |
@kissu Yeah thanks, we'll see what the maintainers of create-nuxt-app will have to say after considering both pros and cons of using Prettier via ESLint. (There has been a very low activity here over last few months, as I still have several pending PRs.) I'm keeping this PR disabled for now, as I haven't tested the ESLint behavior & compatibility myself except the matter of ESLint conflicts, so there may be additional changes needed. I won't be working on this, so feel free to join in the PR if you have updates. We could even consider something like updating this PR to add an optional setting like "use standalone prettier". I personally believe that, at this moment, it's better for the default setup to use standalone Prettier based on its official recommendation, as I haven't observed any critical issues on the latest version. The IDEs that you've mentioned are all covered by https://prettier.io/docs/en/editors.html so if there are any specific cases that aren't covered, it'd be nice to explicitly communicate them, probably even linking them as a GitHub issue for Prettier. It's quite likely that some combinations (Prettier or ESLint) will break again in the future, and it feels like Prettier will only provide up-to-date first-class support if we follow their official setup. I think we should track the specific issues (including difference in performance of the plugin choices, though sometimes a dedicated plugin provides additional benefits - after all, they continuously update it). Btw. your example includes both
This choice will also impact my PR #829 where the prettier would probably disappear from lintfix, so I'd prefer to get it merged first, and then this PR could remove the entry if we decide to go this way (so it's clear that it was intentional).
|
https://github.com/prettier/eslint-plugin-prettier#recommended-configuration
If you use the default
prettier
extension instead of the recommendedplugin:prettier/recommended
and use prettier together with eslint, it can cause issues, for example with the following code:if you run
npm run lint:js -- --fix
, you will geterror Arrow function should not return assignment no-return-assign
instead of the code being fixed to
;[].forEach((b) => (a = b))
as it's supposed to.Prettier officially recommends this setup, so I believe it to be a better default & long-term choice for new Nuxt developers.
An example of a use-case:
https://github.com/quicktype/quicktype#generating-code-from-json-schema
generates code like
typ.props.forEach((p: any) => map[p.json] = { key: p.js, typ: p.typ });
in functionjsonToJSProps
, increasing learning curve by forcing users to look for workarounds once they include a similar code generator to their project.