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

Prettier VS Code 3.0 #958

Closed
13 tasks done
ntotten opened this issue Sep 13, 2019 · 34 comments
Closed
13 tasks done

Prettier VS Code 3.0 #958

ntotten opened this issue Sep 13, 2019 · 34 comments
Labels
discussion locked Please open a new issue and fill out the template instead of commenting.
Milestone

Comments

@ntotten
Copy link
Member

ntotten commented Sep 13, 2019

Yesterday, I spent some time talking with @BPScott about how he and other developers (at Shopify and other places) use VS Code Prettier with the linters. After our discussion, we believe we have a path forward that will allow continued support for prettier + linters while also reducing the issues with the current implementation. Along with that change, we are proposing a few other changes to this extensions. Below are the main changes proposed for VS Code Prettier 3.0. Please let me know what you think.

No longer ship with linters in the extension

Shipping with the linters in the extension results in a lot of potential version conflicts. For example, we currently include prettier-eslint in the extension, however that is not compatible for eslint versions > 5. As a result, many people are installing eslint@6 and seeing this extension break. There isn't really a great way to inform people of those type of conflicts.

In order to make it easier for developers to understand when different dependencies are incompatible, we will require you to install prettier, the linters, and the prettier/linter integrations in your local project (package.json) in order to use the Prettier VS Code + Linter features.

What this means is the linting functionality this extension provides will continue to work, however, in order to set it up you will have to do an NPM install. The benefit of this is if you do an NPM install on incompatible package versions, NPM will tell you and users can better understand the error.

Encourage use of local prettier

We believe that when using prettier in your IDE you should always have a scriptable way of executing prettier as well. The Prettier VS Code integration is really a convenience, but it should not be used on its own. Every project using prettier should support running prettier through a CLI (outside of the IDE) and ideally use something like precommit hooks to ensure that all code checked into source control will be formatted the same.

We will continue to ship prettier with the extension so that users have an easy onboarding experience and so they can use prettier for things like formatting JSON outside of a project, but we will change the documentation to encourage that everyone always installs prettier locally in their projects. We may also provide warnings in VS Code in certain circumstances if you are using the built-in version of prettier.

Reduce Duplicate Configuration

Today, you can configure prettier in VS Code settings, using the .editorconfig file, and using the Prettier configuration files. Additionally, the extension tries to actually merge these version configurations together and applies them in a certain priority order. We have found that this results in a lot of confusion. For example, if you set the global setting to use tabs, but then you clone a project that already has a prettierrc or editorconfig file that overrides this setting, you may not understand where the setting is coming from.

Supporting all of these types of configurations is also a maintenance issue. Every time prettier changes a setting, people ask us to add it to VS Code settings. Additionally, we get requests to add other settings like ESLint, etc.

As such, version 3.0.0 of this project will ONLY support settings through the default prettier configurations. This includes the .editorconfig and prettier configuration file. All prettier duplicate VS Code settings will be deprecated.

Note, some settings will remain like the ignorePath as those provide configuration that is passed to prettier before it runs.


Work

  • Use prettier getFileInfo to resolve which formatter to use as default instead of VS Code. This will resolve many issues such as not formatting graphql, etc.
  • Use prettier getFileInfo to determine if a file is ignored
  • Do not ship linters with extensions (require package.json install)
  • Remove support for older versions of prettier (<1.10.0) and show error
  • Deprecate vs code configuration and add helper to generate .prettierrc from existing config
  • Add Prettier: Initialize command to add .prettierrc file (or whatever format the user wants)
  • Rewrite logger to improve debugging and error handling
  • Warn when using old version of prettier
  • Warn when using invalid linter/prettier combinations
  • TESTS: Resolution of configuration files
  • TESTS: Resolution of ignore files
  • TESTS: Linter configurations
  • Automatically resolve the use of linters (no longer require vscode config)

Release Plan

Because this is going to release as several beta versions and is going to be a large breaking change, I am going to release this version as a new publish (prettier). This is also something we have wanted to do anyway for a while to make it more official.

Initially, I will maintain both versions 2.x on the current publisher and version 3.x on the new publisher.
Once version 3.0 is stable and sufficiently tested, I will publish a new version of 2.x that will simply depend on the new extension - thus moving everyone to the new extension.

@kachkaev
Copy link
Member

but we will change the documentation to encourage that everyone always installs prettier locally in their projects

¢¢: Totally agree that prettier should be installed locally in repos with web apps. However, this is not necessarily true for other languages. For example, if you want to use Prettier with Elm, you'd probably prefer a global instance of prettier and prettier-plugin-elm to package.json + node_modules in the project folder. I'd imagine that this would also apply to folks writing in Java, PHP, PostgreSQL, Ruby and Swift.

All prettier duplicate VS Code settings will be deprecated.

Do you mean flagged as deprecated (still working) or totally removed?


Overall, I fully support the idea that the VSCode extension is just s thin wrapper that calls Prettier CLI for us. Atom Extension for Prettier made a similar shift some time ago.

@BPScott
Copy link
Member

BPScott commented Sep 13, 2019

In order to make it easier for developers to understand when different dependencies are incompatible, we will not require you to install prettier, the linters, and the prettier/linter integrations in your local project (package.json) in order to use the Pretter VS Code + Linter features.

I think there's an extra "not" in there - the plan is to require a local install of prettier, linters and their integrations :)


One potential thing to be aware of: At Shopify we use eslint-plugin-prettier to format our code as part of running eslint. When running yarn run lint from the CLI this runs eslint (which then runs prettier as an eslint rule). However in VSCode we configure the prettier-eslint integration (which runs prettier then eslint over our files) for ease of trivial configuration - telling prettier-vscode to format every file is simpler than having to configure VSCode to use vscode's eslint extension to format js/ts files and prettier-vscode to format everything else (this is actually slightly more complex as we also deal with stylelint for css file too but the concept is the same). It would be useful if there was a way to keep this simple configuration without us needing to install a local copy of prettier-eslint - where this plugin could either use a global version of prettier-eslint if it identifies a local version of prettier and eslint are installed (if a global prettier-eslint can use local versions of those deps) or have some config option where formatting certain files types would call eslint --fix directly.


¢¢: Totally agree that prettier should be installed locally in repos with web apps. However, this is not necessarily true for other languages. For example, if you want to use Prettier with Elm, you'd probably prefer a global instance of prettier and prettier-plugin-elm to package.json + node_modules in the project folder. I'd imagine that this would also apply to folks writing in Java, PHP, PostgreSQL, Ruby and Swift.

That would mean anybody with non-node etc projects will run into problems with shared dependencies. Consider having local checkouts of php-project-A which is formatted in the style that prettier v1.8 provides and php-project-B which is formatted in the style that prettier v1.15 provides. Using a global install of prettier it would be impossible to work on both of these projects simultaneously without accidentally reformatting one to use not-per-project conventions. You'd have to change the globally installed version of prettier constantly. Project-level isolation is vital for linting tools even if your linting tool isn't written in the same language as your base project.

Do you mean flagged as deprecated (still working) or totally removed?

I'd guess existing but undesired behaviour (e.g. defining prettier config within vscode) would be deprecated in v2.x releases then removed v3.0.

@kachkaev
Copy link
Member

kachkaev commented Sep 13, 2019

Consider having local checkouts of php-project-A which is formatted in the style that prettier v1.8 provides and php-project-B which is formatted in the style that prettier v1.15 provides

I'm aware of this limitation. However, I still see benefits in using global prettier and prettier plugins in certain scenarios. One concrete example would be education: a student that works on their own (non-js) code uses the same instance of prettier and prettier-plugin-x no matter what file on their disk needs formatting. No collaboration and hence no prettier version clashing takes place. That's the case for litvis, which benefits from prettier + prettier-plugin-elm. Students are not taught JS, NPM and the rest of the ecosystem – that's too hard. Instead, they are focused solely on writing Markdown and Elm. Running npm install --global prettier prettier-plugin-elm is a one-off command they need to run without understanding what that's doing.

@ntotten
Copy link
Member Author

ntotten commented Sep 17, 2019

Do you mean flagged as deprecated (still working) or totally removed?

I was thinking a bit about this. I think what I would like to do is mark them as deprecated and then prompt a warning for people who are using them and link to a doc. We would not use the settings for prettier, but this would make it so people can migrate and understand what is going on. Then in say version 4.0 we would remove them entirely.

@ntotten
Copy link
Member Author

ntotten commented Sep 17, 2019

I think there's an extra "not" in there - the plan is to require a local install of prettier, linters and their integrations :)

Yes. Fixed. :)

@Kureev
Copy link

Kureev commented Sep 20, 2019

Anything community can help you with? I'm interested in upgrading eslint to 6.x and would love to invest some time to make it possible without breaking prettier-vscode :)

@ntotten
Copy link
Member Author

ntotten commented Oct 20, 2019

Started working on this in a branch: https://github.com/prettier/prettier-vscode/tree/3.0

@ntotten
Copy link
Member Author

ntotten commented Oct 20, 2019

@Kureev The eslint 6 issue isn't a bug in this extension directly, its in a dependency library. See #870

@ntotten ntotten added this to the Version 3.0 milestone Oct 21, 2019
@ntotten ntotten mentioned this issue Oct 21, 2019
3 tasks
@robwise
Copy link

robwise commented Oct 21, 2019

These are really good changes.

In prettier-atom we've already switched to using getFileInfo and it's working much better (and is required if you want to start supporting plugins). Honestly, this is how most linter packages work (you're expected to configure everything so that the project drives the linting settings).

We ripped out the ability to configure their prettier options via the package a while ago. You will have some people complain at first about how they can no longer configure everything through the package and how it was so easy before, but it's not your fault, it's the direction that Prettier went (and IMO, the correct direction) from a simple couple of options to something that is ever-growing and more enhanceable with plugins. It's just plain not compatible with the in-editor settings anymore (and we had the same problem where we were constantly having to play catchup with new options Prettier was adding).

If they still want a default/global configuration, they can just put a prettierrc in their root folder.

Also, we've had that same issue with prettier-eslint, and honestly I'm seriously considering pulling it out although some users are really insistent that they still want it. There is a fix for that coming very soon they just have to merge it (they weren't respecting the current working directory).

And .editorconfig can be picked up automatically by Prettier so you don't have to worry about it yourself so again that's not something the editor packages should have to worry about IMO.

@ntotten
Copy link
Member Author

ntotten commented Oct 21, 2019

@robwise Awesome, this is really good to hear. Thanks for letting me know!

@ntotten
Copy link
Member Author

ntotten commented Oct 22, 2019

The preview version 3.0.0 is now live on the marketplace for anyone who wants to try it. There will be bugs and there are still more breaking changes coming, so I wouldn't use it as your main formatter yet. You can use VS Code insiders and install this extension to keep your main VS Code clean. https://marketplace.visualstudio.com/items?itemName=Prettier.prettier-vscode

@leepfrog
Copy link

Thanks for sharing @ntotten.

FWIW: this solved my linting woes on my project where I'm using eslint 6.5.1 / typescript 3.7.0 beta w/ new experimental features (using the un-merged version of prettier/prettier#6657).

@YuriScarbaci
Copy link

As such, version 3.0.0 of this project will ONLY support settings through the default prettier configurations. This includes the .editorconfig and prettier configuration file. All prettier duplicate VS Code settings will be deprecated.

if i have no configuration and I only want the extension to use the eslint ruleset, will I have to change anything to comply with version 3? will I have to duplicate the eslint ruleset inside .editorconfig/.prettierrc ?

@ntotten
Copy link
Member Author

ntotten commented Oct 27, 2019

@YuriScarbaci You shouldn't, however, if you want to publish a github repo of your setup I can use it to validate that things are working as expected.

@YuriScarbaci
Copy link

@ntotten

sure, this is a basic react repo i created as a demo with only the eslint config i'm currently using in my other projects:

https://github.com/YuriScarbaci/react-eslint-bootstrap

currently I am not using prettier-vscode anymore due to the feature of reading the ruleset from .eslint not working with the latest eslint versions, but, truth be told, I'm missing the good old prettier extension that was able to format my document in one go instead of having to eslint-fix multiple times. + prettier was formatting the document even if there was an error in the eslint, right now i must first fix the error and then i can eslint-fix format...

@ntotten
Copy link
Member Author

ntotten commented Nov 3, 2019

@BPScott I'm getting close to finishing up version 3.0. Would you mind verifying the preview version of 3.0 and see it there are any issues with the shopify setup or if you have any other feedback/issues. You can install the preview version here: https://marketplace.visualstudio.com/items?itemName=Prettier.prettier-vscode

@tleunen
Copy link

tleunen commented Nov 5, 2019

@ntotten - Since I installed v3, I'm getting this error even though I believe I removed all legacy settings?

You have legacy linter settings in your VS Code config. They are no longer being used. See documentation for migration information

The only prettier config I have in my settings is "prettier.requireConfig": true. This might also be legacy? Anyway. Even without it the error persists.

And it seems that eslint/tslint doesn't autofix anything/everything anymore even though I have editor.formatOnSave and eslint.autoFixOnSave set to true. Although it might not be a problem of prettier at this point since prettier doesn't run these from what I understand.

@ntotten
Copy link
Member Author

ntotten commented Nov 5, 2019

@tleunen the prettier.requireConfig is not legacy so that shouldn't be it. Is it possible you have another prettier setting configured in your global user settings? I probably need to make the error better to indicate where we found the settings. I'll see if I can reproduce the error tonight.

As far as the eslint stuff goes, we still do support the prettier-eslint integration, but there aren't settings for it anymore. See the new way to set it up here: https://github.com/prettier/prettier-vscode/tree/3.0#vs-code-eslint-and-tslint-integration under the heading Prettier Linter Integration (advanced)

@tleunen
Copy link

tleunen commented Nov 5, 2019

Good catch. I forgot I had a workspace settings file in my project 😅
Thanks. I'll make more tests asap with these tools.

@BPScott
Copy link
Member

BPScott commented Nov 6, 2019

@BPScott I'm getting close to finishing up version 3.0. Would you mind verifying the preview version of 3.0 and see it there are any issues with the shopify setup or if you have any other feedback/issues

Will do, I'll try and squeeze some time in later this week.

@David-Else
Copy link

Prettier 1.19.1 is just out! Vital support for TypeScript 3.7 is now available!!

I was wondering how long until this gets into Prettier VS Code 3.0? Thanks!

If I delete my current version and add the new version from https://marketplace.visualstudio.com/items?itemName=Prettier.prettier-vscode am I OK from now on? That will become the official version, or do I need to monitor it and do something else when it goes stable?

https://prettier.io/blog/2019/11/09/1.19.0.html

@ntotten
Copy link
Member Author

ntotten commented Nov 10, 2019

@BPScott I see that in your referenced issue above you are switching to use eslint for prettier formatting. To clarify, I am not removing the support for linters in this version of the extension, but maybe if the eslint extension is better for that type of integration we should. Let me know what you think.

@David-Else
Copy link

Just updated to prettier.prettier-vscode. It correctly alerted me to update my settings.json and created a new config file. I copied that into my package.json (as it is neater) and now everything seems to work awesome!

  "prettier": {
    "singleQuote": true,
    "proseWrap": "always",
    "trailingComma": "all",
    "quoteProps": "consistent"
  },

Big improvement to use Prettier's own settings rather than VS Code's, that was always a hack. Things feel more cruft free and groovy now... THANKS!

@David-Else
Copy link

I think I have a bug, it does not seem able to format JSON with comments. Add this, then you get an error:

  "[jsonc]": {
    "editor.defaultFormatter": "prettier.prettier-vscode"
  },

@ntotten
Copy link
Member Author

ntotten commented Nov 11, 2019

@David-Else I’ll check into ASAP. Thanks for reporting.

@BPScott
Copy link
Member

BPScott commented Nov 11, 2019

@BPScott I see that in your referenced issue above you are switching to use eslint for prettier formatting. To clarify, I am not removing the support for linters in this version of the extension, but maybe if the eslint extension is better for that type of integration we should. Let me know what you think.

unsure grunting and huffing. It's complicated and my hand was kinda forced by upstream inaction and the passage of time.

In the above linked issue we updated to eslint 6 which is not currently supported by prettier-eslint and while the PR to add eslint 6 support looks trivial prettier-eslint's maintainer hasn't been able to find the time to merge it (and that's totally OK!). I found that trying to format code using esbenp.prettier-vscode with eslintIntegration enabled with eslint v6 installed within my project caused errors when attempting to format files - which i suspect is due to eslint version mismatches between the version bundled with the plugin and the my project.
I didn't want to wait for prettier-eslint any longer so i went with switching to use the eslint plugin for autoformatting my js/ts files.

In an ideal world I'd want some symmetry between how my js, css and md files are formatted. In the past this was easy - they were all formatted using esbenp.prettier-vscode with the eslint and stylelint integrations enabled, this used prettier-eslint and prettier-stylelint under the hood However both of those projects have maintainers that are focusing on other things (and that's ok) and that means that even though they're pretty low maintenance waiting on updates to add support for new major versions of eslint/stylelint is becoming painful - eslint 6 was released in June, and a PR to update prettier-eslint to support eslint 6 has been open since August, while prettier-stylelint hasn't been updated in 2 years.

In a new world I'd also be happy with the removal of linter support as long as there's a consistent story for each of those file types. The dbaeumer.vscode-eslint plugin solves the job for formatting js/ts files on save, replacing the need for the eslintintergration option. However the most popular stylelint plugin shinnn.stylelint doesn't support autofix. There's young upstart that forked that extension and added autofix support in a similar way to the eslint plugin: hex-ci.stylelint-plus but I've not yet had a chance to try it out to see if it suitable to remove my dependency on the stylelintIntegration option.

Keeping support for linters for the time being would be helpful till I can confirm that's there's a good story for "you want to autoformat your css/scss/etc files using stylelint (which runs prettier as a plugin)." that will allow me to no longer need the stylelintIntegration option.


An alternative to the endpoint of using multiple plugins for the varying file types we could keep the integration settings and stick with the rule of "esbenp.prettier-vscode formats everything" but instead of using prettier-eslint and prettier-stylelint for js/ts/css/scss/etc files this plugin runs eslint / stylelint itself after running prettier - essentially becoming responsible for that "prettier then $OtherFomattingTool" formatting flow instead of delegating to prettier-eslint / prettier-stylelint. This would be a bit simpler from a consumption perspective as consumers wouldn't have to rely on multiple plugins, and would mean that consumers would be able to format documents without saving (dbaeumer.vscode-eslint and hex-ci.stylelint-plus don't implement their formatting as a VSCode formatter so you can't do "cmd+shift+p => Format document" to format a file). But that feels a bit like the kind of feature creep that you've been trying to avoid.


Oh and just for clarity: I haven't had a chance to try out v3 package prettier.prettier-vscode yet, sorry. I wanted to get my project into a state where it worked with our newest eslint configs before attempting the new prettier package.

@BPScott
Copy link
Member

BPScott commented Nov 11, 2019

Prettier 1.19.1 is just out! Vital support for TypeScript 3.7 is now available!!

I was wondering how long until this gets into Prettier VS Code 3.0? Thanks!

@David-Else it is my understanding that this plugin uses the version of prettier that is installed within your own project (and only falling back to its bundled version if that isn't found). You should be able to update your package.json to depend upon the latest version of prettier and you'll be good to go.

@flatherskevin
Copy link

@BPScott This doesn't seem to be the case. See the issue I opened regarding it #1027

@David-Else
Copy link

Prettier VS Code 3.1 is using the latest prettier:

	"name": "prettier-vscode",
	"displayName": "Prettier - Code formatter",
	"description": "VS Code plugin for prettier/prettier",
	"version": "3.1.0",
...
	"dependencies": {
		"prettier": "^1.19.1",
	},

@ntotten
Copy link
Member Author

ntotten commented Nov 11, 2019

@David-Else I am not able to reproduce the bug with the jsonc settings. If you could open a new issue and provide the full output logs for prettier or share a sample project that would be helpful. Thanks!

@ntotten ntotten closed this as completed Nov 11, 2019
@ntotten ntotten unpinned this issue Nov 11, 2019
@David-Else
Copy link

@ntotten I can't find any output logs for prettier since the icon disappeared from the bottom right panel... it was good to know if prettier was working on a particular file, kind of miss that icon.

@ntotten
Copy link
Member Author

ntotten commented Nov 11, 2019

@David-Else From the view menu click "Output" then from the dropdown there select "Prettier".

I think the icon wasn't really needed after VS Code allowed you to specify the default formatter, but I may add it back if i have some time.

@David-Else
Copy link

@ntotten I have created an issue as you requested #1032

rk-for-zulip added a commit to rk-for-zulip/zulip-mobile that referenced this issue Nov 19, 2019
New versions of the Prettier VSCode plugin (in particular, 3.8) have
started to complain about certain out-of-date settings in VSCode
config files.  Remove the relevant (or rather, now-irrelevant) config
option from our workspace.

Users may need to manually update their Prettier VSCode extension to
continue to format code with Prettier on save.

See:
- https://github.com/prettier/prettier-vscode/blob/62004a02b4/README.md#user-content-error-messages
- prettier/prettier-vscode#958
@prettier prettier deleted a comment from Slooz Feb 18, 2020
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot added the locked Please open a new issue and fill out the template instead of commenting. label May 18, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion locked Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

No branches or pull requests

10 participants