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

feat(eslint): support yarn2 PnP projects #1777

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

williamboman
Copy link
Contributor

@williamboman williamboman commented Mar 14, 2022

https://yarnpkg.com/features/pnp

Yarn's PnP feature changes the way packages are installed. Instead of building on the node_modules resolution, it introduces a single .pnp.*js file in the project. This file is responsible for orchestrating and resolving dependencies. The eslint LSP server will assume that regular node_modules resolution applies when locating the eslint package - which will not work in Yarn PnP projects.

To work around this, Yarn provides the ability to run Node programs in "PnP-compat" mode via yarn exec and yarn node. My understanding is that this simply hooks into the require() function to resolve modules via PnP instead Node's builtin module resolution.

Comment on lines +112 to +104
local pnp_cjs = util.path.join(new_root_dir, '.pnp.cjs')
local pnp_js = util.path.join(new_root_dir, '.pnp.js')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Globbing might be nicer here, but wanted to avoid vim.fn call.

Choose a reason for hiding this comment

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

btw, this is not working with experimental .pnp.loader.mjs which is tries to load vscode-eslint-language-server esm format, and fails because of cjs module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, do you happen to know whether simply adding .pnp.loader.mjs here would do it?

Choose a reason for hiding this comment

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

Yes it would help, but because of this issue it doesn't work, but its not related to this PR

@justinmk
Copy link
Member

Is this ready for merge?

@williamboman
Copy link
Contributor Author

Is this ready for merge?

Yep! I just tested it again in a PNP workspace, works fine (and doesn't work without this).

@justinmk justinmk merged commit 06161ec into neovim:master Jul 5, 2022
@williamboman williamboman deleted the feat/eslint-yarn2-pnp branch August 15, 2022 17:37
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

3 participants