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

Fix plugin search on pnpm #9167

Closed
wants to merge 4 commits into from
Closed

Fix plugin search on pnpm #9167

wants to merge 4 commits into from

Conversation

fisker
Copy link
Member

@fisker fisker commented Sep 8, 2020

Local tested, it not easy to test, because this logic need plugin installed in same location with prettier.

Fixes #8056

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add simple tests?

@fisker
Copy link
Member Author

fisker commented Sep 8, 2020

To setup this test, I need install pnpm, init a project, install prettier and one plugin, and run prettier from this project.

Seems not worth to do that.

@fisker fisker changed the title Fix plugin search on `pnpm Fix plugin search on pnpm Sep 8, 2020
@alexander-akait
Copy link
Member

alexander-akait commented Sep 8, 2020

@fisker we can recreate the structure to make sure it works, in the future we can better automate testing, fixes without tests is bad idea

@fisker
Copy link
Member Author

fisker commented Sep 8, 2020

How do we put prettier self into node_modules/.pnpm/prettier@2.1.1/node_modules/prettier/?

@alexander-akait
Copy link
Member

@fisker we can mock node_modules in tests and recreate structure in test/fixtures/node_modules/.pnpm

@aparajita
Copy link

Thanks for this @fisker!

@wighawag
Copy link

Would be great to have this merged, I switched to npm for my app template because of that.
is the tests the pending issue or is there something else ?

@aparajita
Copy link

@wighawag I'm using pnpm with prettier. There's an easy workaround, use the --plugin-search-dir or --plugin command line option.

@wighawag
Copy link

@aparajita I can make it work from the command line,
unfortunately it does not work on vscode plugin, but this fix proposed here might not fix the vscode plugin issue neither.
In vscode I also get the issue for eslint when used in conjunction with prettier

@alexander-akait
Copy link
Member

/cc @fisker What do you think?

@thorn0
Copy link
Member

thorn0 commented Oct 27, 2020

FWIW, plugins and pluginSearchDirs can be specified in the config too. This might help with the VS Code use case.

@wighawag
Copy link

@thorn0 I tried that but without success, there seems to be other problem with the vscode plugin

@fisker
Copy link
Member Author

fisker commented Oct 28, 2020

@wighawag Are you running a global installed Prettier?

@wighawag
Copy link

No I use locally installed in the root of the mono repo.
My current workaround is to install prettier via npm in a _npm folder and use that path in the prettier vscode config
You can see it in this repo : https://github.com/wighawag/purple-warlock/tree/pnpm

@aparajita
Copy link

@wighawag You should file an issue with the eslint plugin maintainer.

@wighawag
Copy link

@aparajita you mean the prettier plugin I suppose

@aparajita
Copy link

@aparajita you mean the prettier plugin I suppose

Oops, yes.

Base automatically changed from master to main January 23, 2021 17:13
Comment on lines +30 to +32
const autoLoadDir =
findParentDir(__dirname, "node_modules/.pnpm") ||
findParentDir(__dirname, "node_modules");
Copy link

@mdurling mdurling Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this will work.node_modules/.pnpm is the default pnpm virtual store directory, but can be overridden by the virtual-store-dir setting in .npmrc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the information, any suggestion?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. It seems the default path (or whatever is specified in pluginSearchDirs) is being resolved relative to process.cwd() which sits inside the pnpm virtual store. Packages in the virtual store aren't stored hierarchically so the concept of findParentDir does not really work. With pnpm, the package hierarchy is captured in the symlink structure, so the plugin searching will need to use the symlink (not the target) somehow. The only way I have gotten plugins to load is by specifying them in .prettierrc using a relative path, e.g. "plugins": ["./node_modules/prettier-plugin-packagejson"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also use eslint plugins which seem to work fine with pnpm ... Looking at the eslint docs/internals it seems they resolve plugins as if you were to require them from the config file.

Copy link

@aminya aminya Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you accept this simple change as is and defer fine-tuned configuration? Prettier doesn't work with PNPM

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prettier does work with pnpm, it just won’t auto load plugins. You can load them manually in the .prettierrc, for example: "plugins": ["./node_modules/prettier-plugin-packagejson"]

@adamhl8
Copy link
Contributor

adamhl8 commented Jul 23, 2021

For anyone following this thread, I've created a PR that hopefully addresses this issue.

#11248

@syed-haroon
Copy link

I am switching from PNPM to NPM :(

@fz6m
Copy link

fz6m commented May 29, 2022

Rename .prettierrc to .prettierrc.js, use absolute plugin path to resolve this problem.

// .prettierrc.js

module.exports = {
  plugins: [
    require.resolve('prettier-plugin-packagejson'),
    // ...
  ]
}

@yavko
Copy link

yavko commented Jun 18, 2022

Rename .prettierrc to .prettierrc.js, use absolute plugin path to resolve this problem.

// .prettierrc.js

module.exports = {
  plugins: [
    require.resolve('prettier-plugin-packagejson'),
    // ...
  ]
}

this doesn't work for me

@thonkinator
Copy link

any updates on this? it's been open for over 2 years now

@fz6m
Copy link

fz6m commented Jan 8, 2023

Currently the vscode prettier plugin supports resolve pnpm symlinks.

So you don't need require.resolve anymore.

// .prettierrc
{
  "plugins": ["some-plugin"]
}

@thonkinator
Copy link

@fz6m really? for me it doesn't detect pnpm packages either

@henrikvilhelmberglund
Copy link

This was broken for me and I had to migrate from pnpm to npm (loading plugins explicitly didn't work)

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.

[Bug?] Symlink'd plugins in node_modules are not auto detected.