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

try to resolve eslint relatively to nodePath #43

Merged
merged 1 commit into from
Jan 25, 2020

Conversation

BenoitZugmeyer
Copy link
Contributor

Currently, coc-eslint tries to reslove the eslint module contained in a node_modules folder. This patch tries to resolve it locally first, then from the first parent-sibling node_modules folder.

Motivation

Yarn 2 (soon-to-be-released) is leveraging its "Plug'n'play" concept to the next level. Basycally, it doesn't store node modules in the node_modules folder anymore. As a workaround for tooling depending on node modules installed as a project dependency, it creates some modules files in a folder (currently named .vscode/pnpify/eslint). By setting nodePath to this folder, the extension should be able to resolve the needed source and binaries. The VSCode extension handle it correctly, but coc-eslint differs a bit and don't try to resolve the path relatively to nodePath.

Currently, coc-eslint tries to reslove the eslint module contained in a
`node_modules` folder.  This patch tries to resolve it locally first,
then from the first parent `node_modules` folder.
@chemzqm chemzqm merged commit 4aa068c into neoclide:master Jan 25, 2020
@BenoitZugmeyer
Copy link
Contributor Author

@chemzqm Thanks for merging my PR! Would you mind releasing it soon?

@BenoitZugmeyer BenoitZugmeyer deleted the support-yarn2 branch January 31, 2020 15:31
@chemzqm
Copy link
Member

chemzqm commented Jan 31, 2020

Fixed

@chemzqm
Copy link
Member

chemzqm commented Oct 1, 2020

@BenoitZugmeyer I can't understand how could eslint resolved from './eslint' directory, any documentation?

@BenoitZugmeyer
Copy link
Contributor Author

BenoitZugmeyer commented Oct 1, 2020

Here is the related yarn documentation: https://yarnpkg.com/advanced/editor-sdks .

Basically, yarn 2 will not store eslint in the node_modules folder but rather in an arbitrary folder (named .yarn/sdks). Users of yarn 2 would like to specify the eslint.nodePath setting to indicate where to find the eslint module.

Now, the issue is: the coc-eslint resolveModule function will work like the Node.js require.resolve function (this is over-simplified but you get the picture):

  • specifying "eslint" will look into node_modules folders relative to the paths given as argument
  • specifying "./eslint" will look directly into the folders given as argument

So, if we use only resolveModule("eslint", ...) the ESLint install has to be in a node_modules folder. This breaks eslint resolution if it is not in a node_modules folder, no matter what eslint.nodePath is pointing to.

Here is how the vscode extension is handling it: https://github.com/microsoft/vscode-eslint/blob/master/server/src/eslintServer.ts#L761-L763 . It starts to resolve the eslint folder relative to the nodePath setting, then uses whatever paths it determined for traditional package manager installs (namely, ./node_modules folder)

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

2 participants