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

Old version of Prettier is incompatible with VSCode extension #4998

Closed
2 of 17 tasks
weslord opened this issue Jan 17, 2021 · 14 comments
Closed
2 of 17 tasks

Old version of Prettier is incompatible with VSCode extension #4998

weslord opened this issue Jan 17, 2021 · 14 comments
Labels

Comments

@weslord
Copy link
Contributor

weslord commented Jan 17, 2021

Most appropriate sub-area of p5.js?

  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)
    -> Developer tools / environment

Details about the bug:

  • p5.js version: 1.1.9
  • Web browser and version: N/A
  • Operating System: Windows, MacOS
  • Steps to reproduce this:
  1. Install dependencies with npm ci
  2. Open project directory in Visual Studio Code
  3. Install recommended extensions as prompted by .vscode/extensions.json
  4. Run command Format Document in VS Code

The plugin "Prettier Formatter for Visual Studio Code" will display the following error/warning:

Your project is configured to use an outdated version of prettier that cannot be used by this extension. Upgrade to the latest version of prettier.

The tricky thing is that Prettier 2.0 has come out in the meantime. They've introduced a few breaking changes to the formatting, such as using trailing commas and inserting a space before the parentheses in function () {}. These changes require a bunch of small formatting changes to nearly all of the modules in the project.

Unfortunately, despite the error message, the VS Code Format Document command proceeds to format anyways, but it uses a recent version of Prettier rather than the local project version. Since the linting step uses the local version (1.10.2), this means running Format Document can introduce a bunch of formatting problems and cause npm run grunt to fail. (NOTE: This might just be due to how I have VS Code set up personally, falling back to my global settings or something? Can anyone else confirm if this is universal?)

If someone can confirm that this isn't just my system, I think it's worth updating Prettier to save new contributors the potential frustration of formatting errors that are caused by their formatting tools. Thoughts?

References:

https://github.com/processing/p5.js/blob/main/.vscode/extensions.json#L8
https://github.com/processing/p5.js/blob/main/package.json#L81
prettier/prettier#3845

@weslord weslord added the Bug label Jan 17, 2021
@weslord
Copy link
Contributor Author

weslord commented Jan 17, 2021

See main...weslord:prettier-2 for branch with potential PR.

  • 5427522 for config updates
  • 75b9cb4 for formatting updates (mostly autogenerated with lint:fix, inline documentation examples updated manually)

@limzykenneth
Copy link
Member

Would this be something that the VSCode extension be asked to support since as you mentioned there are code style changes between prettier v1 and v2, it usually isn't something projects that uses something opinionated like prettier wants to change without good reasons.

@weslord
Copy link
Contributor Author

weslord commented Jan 19, 2021

It doesn't look like the plugin maintainers are open to supporting old versions:

I can’t support all versions of prettier forever.
(from prettier/prettier-vscode#1122 (comment))

Neither does it look like Prettier is open to making the space in function () a user-configurable option. It was a pretty contentious change with hundreds of comments spread across multiple issues (prettier/prettier#1139 and its children), but the official position is pretty clear: they've changed their opinion.

I did notice that a few of the other major changes were just changes to default settings, so I've changed those settings in my example branch back to the old defaults, making for a slightly smaller diff.

Personally I don't agree with the change, considering how many projects use Prettier and how it will impact git blame, but in p5's case using the old version will lead to more confusion and pitfalls for new contributors.

@weslord
Copy link
Contributor Author

weslord commented Jan 19, 2021

I've done a little more digging, and good news: Looks like the extension supports versions of Prettier back to 1.13.0 (https://github.com/prettier/prettier-vscode/blob/main/src/ModuleResolver.ts#L20), so we don't need to upgrade all the way to Prettier 2 quite yet.

@weslord
Copy link
Contributor Author

weslord commented Jan 19, 2021

main...weslord:prettier-1.19.1 for example of changes required to get to last version before 2.

Still a bunch of changes:

53 files changed, 175 insertions(+), 191 deletions(-)

But much much less than going to Prettier 2:

220 files changed, 5558 insertions(+), 5608 deletions(-)

@limzykenneth
Copy link
Member

For me the changes are pretty excessive, especially around adding brackets to multiplication/division/etc (including a three level deep nesting).

For me personally (which do not reflect the opinion of the project as a whole) I don't see that much value in using prettier if the rules changes that significantly even under the same major version. Code styles should have been a set and forget thing but if sometime down the line prettier changes the rule again and some extension drop support for older version again, we will have to change code style again.

I don't want to get too rant-y here though. This isn't a decision I want to make or should make on my own, I'll defer to other opinion from other contributors.

@stalgiag
Copy link
Contributor

What purpose is prettier serving? I don't use it in personal projects so maybe I am missing something but I think ESLint can auto-fix all of our style rules now.

@limzykenneth
Copy link
Member

Prettier in our case is only used as a rule set that we send to eslint that actually does the linting and fixing for us, so in essence we are just using the code style.

@stalgiag
Copy link
Contributor

I see. Do we use any style rules that can't be managed by eslint? I can't think of any off the top of my head. Single quotes, indent, and max line length can all be managed by eslint without prettier.

@limzykenneth
Copy link
Member

I don't think so, since eslint is pretty comprehensive on its own and prettier is more a case of having something we can point to and say we are following that particular style.

I seemed to remember the documentation mentioning the Airbnb style before we switch to prettier.

@stalgiag stalgiag mentioned this issue Feb 21, 2021
3 tasks
@lmccart
Copy link
Member

lmccart commented Mar 10, 2021

it seems reasonable to me to lose prettier and use eslint

@outofambit
Copy link
Contributor

there's a way to configure prettier to play nicer with eslint but i agree with @limzykenneth and @lmccart above. for the purposes of this project prettier is probably not worth the overhead if we get some basic style rules in our eslint config.

we could also do a quick upgrade of prettier to 1.13 or something higher but still below 2.0 in the meantime but again maybe not really worth the effort

@jesi-rgb
Copy link
Member

jesi-rgb commented Jul 3, 2022

Hi! Stumbled upon this issue looking for the keyword linter in the issues, since I am experimenting the exact problem here described.

As @limzykenneth mentioned, it should be a "set and forget", but as I am now trying to implement some functionality, 90% of the time has been spent dealing with the VSCode prettier extension yelling that it is an old version. I have to reconfigure prettier and restart VSCode only to find the same errors or even different ones.


For me the changes are pretty excessive, especially around adding brackets to multiplication/division/etc (including a three level deep nesting)

In #5694, @limzykenneth precisely pointed this out to me and that is exactly what happens. If I didn't add the extra brackets, the commit github action would tell me to add them.

I'll see if I can get rid of prettier whilst still having eslint auto format my code, since adding spaces and tabs is, in my humble opinion, an entirely copmuter based task.

@Qianqianye
Copy link
Contributor

We have removed prettier dependency, thanks to @limzykenneth. We have also removed it from vscode extension. So I will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants