Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: setup Prettier with ESLint #121

Merged
merged 8 commits into from
Dec 12, 2024

Conversation

antfu
Copy link
Contributor

@antfu antfu commented Nov 5, 2024

#117 #118

This PR setup Prettier to use alongside with ESLint.

  • Disable stylistic in ESLint
  • Install ESLint, Prettier, and the config as dev deps
  • Install TypeScript as dev dep; Nuxt ESLint needs it to support linting TypeScript files.
  • Add two scripts for CLI linting/formatting

I didn't update the npm-lock, so CI might fails. After installing the packages, you might want to restart VS Code to have ESLint effective. Meanwhile there are some linting and formatting can be applied by running npm run format that I didn't cover in this PR

@antfu antfu changed the title fix: setup Prettier with ESLint build: setup Prettier with ESLint Nov 5, 2024
@drewbaker
Copy link
Member

@antfu thank you so much for this! I tested it out today, and it works except it doesn't seem to have any of the Vue recommended rules.

Can you add in the Vue3 recommended rule sets? Getting those to work and not conflict with Prettier and ES Lint is very confusing.

It also worth mentioning that it seems to require these two VS Code plugins to work, which is fine, just documenting it here for future:
https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode
https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint

@antfu
Copy link
Contributor Author

antfu commented Nov 10, 2024

What rules do you want to include? Nuxt by default already includes many Vue rules:
Screenshot 2024-11-10 at 11 28 42

@drewbaker
Copy link
Member

Just the Vue recommended set. Perhaps I have this wrong, it didn’t appear to be doing the basics like attributes on multiple lines when I looked.

https://eslint.vuejs.org/rules/#priority-c-recommended-potentially-dangerous-patterns

@drewbaker
Copy link
Member

Yeah, so I tried it with a more complicated component, and I get a flash of formatting, then back to what it was.

Here is a video of what I see:
https://github.com/user-attachments/assets/834139ed-5f70-44bc-8ea0-8e1b0b94cf67

@antfu
Copy link
Contributor Author

antfu commented Nov 10, 2024

I see. That's the one of the reason I personally won't recommend using Prettier - it doesn't give you granular control over newlines - and it's fundamentally by design of their approach. I guess you need to make the trade-off here by not using Prettier, or not care about this rule

@drewbaker
Copy link
Member

OK I see. Is it possible to achieve the same thing with ES Lint then? I thought it didn’t really handle CSS?

We did have ES Lint and Prettier working well in the 2.x branch, but that was Nuxt 2.

@antfu
Copy link
Contributor Author

antfu commented Nov 16, 2024

Sorry, I was taking some day offs.

I just pushed a commit that uses ESLint (with ESLint Stylistic) and disabled Prettier. eslint-plugin-format allows to delegate CSS/HTML formatting to an embedded Prettier while leaving the others handled by ESLint - hopefully it would meet your requirements.

Again I didn't commit the lock of formatting to make this PR easier to review - please run npm install and restart VS Code, and run npm run format to see the formatted result.

@drewbaker
Copy link
Member

drewbaker commented Nov 17, 2024

OK so deactivate VS code prettier plugin, but keep the es lint plugin active?

@antfu
Copy link
Contributor Author

antfu commented Nov 17, 2024

Yes, the config should already disabled Prettier so you don't need to do it manually

@drewbaker
Copy link
Member

drewbaker commented Dec 4, 2024

OK, this is now linting Vue template and script blocks, but not working for Vue style blocks.

Also, it is not linting .css, .svg, .json. mjs files. I know ES Lint isn't targeted at CSS, but I thought that was what the internal Prettier was doing?

Again, just to re-state the goal here: Format and fix errors on save for Vue, CSS, HTML, JS, TS, MJS, and JSON files, respecting the Vue recommended style guide rules for Vue, and Prettier for the rest. Stretch goal would be PHP files too.

@drewbaker
Copy link
Member

Something I noticed, from this Stackoverflow post: https://stackoverflow.com/questions/79176202/getting-prettier-and-es-lint-working-on-save-with-nuxt-3/79188343#79188343

If I set my settings.json file to this:

"javascript.validate.enable": true,
"eslint.debug": true,
"eslint.useFlatConfig": true,
"editor.codeActionsOnSave": {
  "source.organizeImports": "never",
  "source.fixAll": "always",
  "source.fixAll.eslint": "explicit",
  "source.fixAll.stylelint": "always"
},
"eslint.validate": [
  "javascript",
  "javascriptreact",
  "typescript",
  "typescriptreact",
  "vue",
  "html",
  "markdown",
  "json",
  "jsonc",
  "yaml",
  "toml"
],

Then my CSS files and blocks will be fixed on save, but Vue template recommended rules don't work then...

@drewbaker
Copy link
Member

Through process of elimination I figured out it was "editor.formatOnSave": false, that would make it work for all the other files. So I suppose then it's using the VS Code internal language features, so basically not ES Lint then? So Not the answer we are seeking.

Man, I have to vent... Why the hell is this so hard?! 😭

@drewbaker
Copy link
Member

So i tried using the exact setup shown here: https://stackoverflow.com/a/79188343/503546

It seems to run on all files now, but with weird results.

  • Vue script blocks go to top of the file now
  • Adding semi-colons to JS and TS files
  • Vue recommended style guides rule not being applied
  • 2 space indents in JSON files, but 4 space in Vue files (we want 4 space indents everywhere)

I'm at a dead end again. Would love your help from here @antfu.

@antfu
Copy link
Contributor Author

antfu commented Dec 11, 2024

I am not sure I understand what you are looking for exactly.

I see:

  • Vue's recommended rules working fine
  • semi being removed in js/ts files
  • Do you want the script block to be on top or not?

And I just installed the JSON plugin for you to add 4 space indent

I committed the formatted result to this branch to better show you the result.

Is there anything missing?

@drewbaker
Copy link
Member

Again, just to re-state the goal here: Format and fix errors on save for Vue, CSS, HTML, JS, TS, MJS, and JSON files, respecting the Vue recommended style guide rules for Vue, and Prettier for the rest.

@antfu
Copy link
Contributor Author

antfu commented Dec 11, 2024

From my test everything you mentioned should be working already, no? (see the diff of this PR for example)

@drewbaker
Copy link
Member

Thanks @antfu for persisting here. After your new changes, I tested the following:

  • Vue files lint + fix, including template, script and style blocks
  • CSS files lint + fix (Prettier or equivalent)
  • SVG files lint + fix (Prettier or equivalent)
  • JS/TS/MJS files lint + fix (Prettier or equivalent)
  • HTML files lint + fix (Prettier or equivalent)

Here is my screen recording showing the tests (except I didn't include HTML file).

Screen.Recording.2024-12-11.at.10.15.33.AM.mov

So just to be really specific, currently these needs to be implemented:

  • CSS files lint + fix (Prettier or equivalent)
  • SVG files lint + fix (Prettier or equivalent)

@antfu
Copy link
Contributor Author

antfu commented Dec 11, 2024

I fixed the CSS part for the format on saving. Should we have this merge and iterate then?

@drewbaker
Copy link
Member

OK, great. Merging, and can add SVG after

@drewbaker drewbaker merged commit a1aa227 into funkhaus:master Dec 12, 2024
0 of 4 checks passed
@drewbaker
Copy link
Member

@antfu Opened issue for SVG linting here #126

@antfu antfu deleted the feat/prettier-eslint branch December 12, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants