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

Custom Prettier Resolution #232

Closed
bradenhs opened this issue Oct 2, 2017 · 22 comments
Closed

Custom Prettier Resolution #232

bradenhs opened this issue Oct 2, 2017 · 22 comments
Labels
enhancement locked Please open a new issue and fill out the template instead of commenting.
Milestone

Comments

@bradenhs
Copy link

bradenhs commented Oct 2, 2017

Instead of needing to have prettier installed in your current folder's node_modules directory it'd be nice to have the option to specify where you'd like the extension to look for your prettier installation. Resolution strategy would be as follows:

  1. If "prettier.path" is defined try to use that.
  2. See if there's a copy in the current directory's node_modules folder.
  3. Use a copy of prettier bundled with the installation.

The issue I ran into: I'm using yarn workspaces and it hoists dependencies if they're shared between different workspaces. Because of this my prettier installation isn't in the individual workspace's node_modules folder. It'd be nice to have the ability to set the following in my .vscode/settings.json file:

"prettier.path": "../../node_modules/prettier"

Thoughts?

@azz
Copy link
Member

azz commented Oct 2, 2017

Seems totally reasonable to me. I think typescript has a similar setting.

@CiGit
Copy link
Member

CiGit commented Oct 3, 2017

If I understood correctly your file system looks like

  • node_modules/prettier
  • sub/project/package.json{"prettier"}

I think we can update our local resolution algorithm to resolve this case. Resolve from package.json's folder.

➕ No new settings (and we wouldn't have to do that for other modules we have)

➖ Still won't resolve a randomly installed prettier (global install, ...)

@bradenhs
Copy link
Author

bradenhs commented Oct 3, 2017

I'd be just as happy with either solution.

@priley86
Copy link

priley86 commented Oct 3, 2017

+1

@Bnaya
Copy link

Bnaya commented Oct 20, 2017

Same issue here, with mono repo

Failed to load prettier from /Users/.../project-root/packages/webapp/node_modules/prettier. Using bundled

yarn workspace is hoisting prettier to project-root/node_modules

@agoldis
Copy link

agoldis commented Oct 22, 2017

another use case that needs that - prettier is hoisted as part of lerna configuration, what left for lerna sub-package is only symlink to hoisted npm module

@kevin-ashton
Copy link

kevin-ashton commented Jan 22, 2018

Any update on this? I think the original proposal seems ideal whereas it provides max flexibility.

As I understand it I don't think the proposal of updating the local resolution algorithm would work on my project. We have a cli tool (that can live anywhere) that handles running various things across projects for us including formatting code. It would be very helpful to be able to specify the path of the prettier executable similar to what the SublimeText plugin does.

@CiGit
Copy link
Member

CiGit commented Jan 25, 2018

The resolution algorithm has just been updated but not yet released. #342

I'd like to avoid adding options. If your project depends on prettier, why not having it inside it's dependencies?
It's not a npm project? (We only resolve those for now)

In the future with more language support (Java / python / ...), It may make sens.
IMO if your project depends on a specific prettier version, you should specify it.

@kevin-ashton
Copy link

kevin-ashton commented Jan 26, 2018

I think creating an algorithm to try and intelligently determine which prettier module to use is reasonable. That being said if you don't have a way of overriding the prettier resolution you are restricting this tool projects that happen to follow the organization patterns your algorithm is expecting.

In my case the opinionated nature of this tool make it unfeasible to use at work and on various side projects I have. A few examples include:

  • On one project at work we have mono repo that branches into N modules. Some of these modules contain typescript and other assets we would like to run through prettier. The node_modules directory and config live within one of these modules for a variety of reasons. For a variety of reasons we don't want it at the repo root. The rational for our file organizational pattern is beyond the scope of this discussion but rather is meant to serve as use case where algorithm doesn't work.

  • Another example deals with various adhoc files and projects I build in my spare time. In these cases I want a consistent formatting options. Granted I could set the options in the settings, but I also tend to switch to various editors (Sublime, Webstorm, etc) depending on what I'm doing. When switching editors I would prefer to have all the editors reference a common .pretterrc file and prettier executable for consistent behavior.

These are just meant as two illustrates but I feel confident additional use cases could be found. Regardless IMO it seems less ideal that the official prettier extension would also appear to be indirectly opinionated about file organizational structure, a topic which has nothing to do with prettier in my mind.

Bottom line is I like the tool and would like to use it but in its current form I can't. Hope a solution can be found.

@CiGit
Copy link
Member

CiGit commented Jan 27, 2018

That's fair.
Even doing an algorithm for most of common project structures out there would be pretty hard. Prettier supporting more and more languages desserves us.

[...] all the editors reference a common .pretterrc file [...]

This would also be an option we would have to add. Prettier cli allows it.

Side effect: It would be faster to format as we now where to search ;-)

@CiGit
Copy link
Member

CiGit commented Jan 27, 2018

What about a 3-state option ?
prettier.localPrettier: true
State:

  • true (default) same as current, search for prettier in package.json
  • false use bundled prettier
  • path: string use given prettier.

Option name up for bikeshedding !

We also would have to think about future plugins. How would it work with them?
We can't bundle all of them.
@azz may have an idea

@kachkaev
Copy link
Member

kachkaev commented May 11, 2018

It'd be great if the package fell back to a globally installed Prettier before using a built-in one, because global Prettier can also go with some plugins installed. Global Prettier plugins will be discoverable soon in 1.13, because prettier/prettier#4000 has been fixed.

I implemented falling back to a global Prettier in Atom, see prettier/prettier-atom#397. Would something similar be acceptable for VSCode too?

@CiGit
Copy link
Member

CiGit commented May 16, 2018

How are you searching for that global prettier?
$PATH ? Known location ?

Asking users to input theirs installation would cover more use case I presume. And speed up the thing.

@kachkaev
Copy link
Member

kachkaev commented May 16, 2018

In Ptrettier for Atom (prettier/prettier-atom#397), I proposed to search for Prettier installed globably via npm and yarn — both of these locations can be easily derived on any os. I don't know of any other method to install Prettier globally as of now. $PATH could work too, it's probably even easier. However, this method can only be used when we are calling Prettier as a CLI tool rather than using its JavaScript API.

Perhaps, letting users to specify a custom copy of prettier could be useful to someone too, but I'm not sure it would be needed by many (perhaps, mainly for developers)

@CiGit
Copy link
Member

CiGit commented May 16, 2018

To install global package, you have to be root on most systems.
You can however create yourself a $HOME/[..]/npmbin project where you install your global tools and tweak your $PATH.
Yes, only the CLI tool is on the path.

Other package manager may work differently tho. (You may even have your distribution packaging it for you)

You could even set your config to point to something which isn't prettier but a fork or an other tool with the same API. (I'm not sure if I should put that in pros or cons ...). We could imagine prettier-**lint wrapper 😄

@kachkaev
Copy link
Member

kachkaev commented May 16, 2018

To install global package, you have to be root on most systems.

This is only the case for Linux, because homebrewed npm on macOS or a bundled Node+npm for Windows don't require admin permissions. Even if admin rights are needed, it's still easier to globally install, say, Prettier and prettier-plugin-php and then use it in every project than to carry 100MB of node_modules in every folder with PHP files. Calling npm install locally does not make sense when the project does not use JavaScript.

Indeed, people can install Prettier globally anyhow they want. If the VSCode plugin uses Prettier via CLI, it will always able to find it (regardless of if it's local or global). Scanning global node_modules by npm and yarn as in prettier/prettier-atom#397 is an alternative approach. It is less universal, but is more performant, according to @robwise. I don't have a strong opinion in terms of how to search for the global Prettier and am happy with any approach.

IMO it makes sense to search for Prettier in this order:

  1. Prettier by a custom path to its cli.js (e.g. when testing a development version of it)
  2. local Prettier
  3. globally installed Prettier (via npm, yarn or just appearing in $PATH)
  4. Built-in Prettier form prettier-vscode (the only one that is guaranteed to not have any plugins)

It should work well for all people and I can't imagine a situation when it surprises someone. If a person wants to use a custom copy of Prettier for a particular project or workspace, they simply change a corresponding settings.json file. Otherwise, customPrettierPath (or whatever the option) is null, so the first step is skipped.

@deppe
Copy link

deppe commented Feb 27, 2019

Another use-case to consider is support for forks like prettier-miscellaneous. I have prettier-miscellaneous installed locally, but the plugin does not seem to detect it and instead uses its bundled prettier install.

@jannikbuschke
Copy link

Is there any workaround at the moment?
Using lerna and prettier gets hoisted. Im suspecting this is why VS code does not apply prettier on save.

@mwilliammyers
Copy link

Another use case is formatting TOML files via prettier-plugin-toml in Rust projects...

@accidentaldeveloper
Copy link

accidentaldeveloper commented Nov 1, 2019

Another resolution scenario to consider is with Yarn PnP/v2. When using Yarn PnP the package is installed in the project but not extracted to node_modules.

@ntotten
Copy link
Member

ntotten commented Nov 2, 2019

The setting has been added in version 3.0

@ntotten ntotten closed this as completed Nov 2, 2019
@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 Apr 12, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement locked Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

No branches or pull requests