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

chore: use prettier for formatting #6096

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Jan 20, 2024

What's the problem this PR addresses?

How did you fix it?

  • Removed the formatting related rules in the eslint config.
  • Added eslint-config-prettier to the eslint config.
  • Added Prettier and formatted the codebase with it.

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@merceyz merceyz requested a review from arcanis as a code owner January 20, 2024 17:27
@merceyz merceyz force-pushed the merceyz/chore/prettier branch 2 times, most recently from 5cab70a to 77fcdcb Compare January 20, 2024 17:58
@arcanis
Copy link
Member

arcanis commented Jan 20, 2024

I understand prettier is a neat tool to remove discussions about the "proper" coding style, but I think the rules we have put us in a nice place already, being the right balance between consistency and flexibility. In that way, I don't believe prettier offers us enough value here that it is worth the change.

The deprecation of the formatting eslint rules is very disappointing, but unless I'm mistaken it's not an immediate issue today and, if push comes to shove, I'd actually write my own rules than forego a coding style I grew accustomed to.

I understand if that looks like an eccentricity, but that's one I feel fairly strongly about...

@merceyz
Copy link
Member Author

merceyz commented Jan 21, 2024

I understand prettier is a neat tool to remove discussions about the "proper" coding style, but I think the rules we have put us in a nice place already, being the right balance between consistency and flexibility. In that way, I don't believe prettier offers us enough value here that it is worth the change.

I disagree, whenever I write code in this repo I either manually nudge the code to get it to match the rest of the file, which is a waste of time, or I force format it with prettier.

I don't care what formatting style we use as long as it's consistent throughout the codebase and our current setup doesn't do that.

Would you be more open to something like https://dprint.dev/ which lets you configure more?

The deprecation of the formatting eslint rules is very disappointing, but unless I'm mistaken it's not an immediate issue today

It's not but it's another sign that maybe using ESLint for formatting isn't a good idea.

I'd actually write my own rules than forego a coding style I grew accustomed to.

That seems a bit extreme, presumably you'd get used to this style and eventually realize you don't actually care.

I understand if that looks like an eccentricity, but that's one I feel fairly strongly about...

Strongly enough to veto a vote on it?

@arcanis
Copy link
Member

arcanis commented Jan 21, 2024

I don't care what formatting style we use as long as it's consistent throughout the codebase and our current setup doesn't do that.

What's an example of inconsistency you have in mind, so we have a baseline?

Would you be more open to something like https://dprint.dev/ which lets you configure more?

Perhaps; I don't want drastic changes, and don't want width-control rules. So it depends on what set of changes you have in mind.

@demurgos
Copy link
Contributor

I feel that a formatter integrated into the repo is very helpful for occasional contributors.

@trivikr
Copy link
Contributor

trivikr commented Feb 20, 2024

If switching to prettier, we should consider using config closest to existing style to reduce diff.
For example, the singleQuote configuration can be set to true.

Another way to incrementally switch to prettier is to only enable it for TypeScript files to start with, and later including other files - like markdown.

Although eslint-stylistic is community supported, there are passionate folks who're committed to maintaining it. Many projects from Vue ecosystem are going to continue using ESLint Stylistic rules. Can yarn stick to ESLint for formatting, to avoid large diff and adding another additional dependencies?

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.

None yet

5 participants