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

Run ESLint from project root dir where possible #2937

Merged
merged 1 commit into from
Jan 1, 2020

Conversation

kevinoid
Copy link
Contributor

As discussed in #2787, ESLint 6 loads all plugins/configs/parsers relative to the project root which, by default, is the directory in which ESLint is invoked, as described in ESLint RFC 2018-simplified-package-loading.

Therefore, ALE should run ESLint from the project root, when possible, so that dependencies will load. This PR makes the change to do so.

Fixes: #2787

Thanks for reviewing,
Kevin

ESLint 6 loads all plugins/configs/parsers relative to the project root
which, by default, is the directory in which ESLint is invoked, as
described in [ESLint RFC 2018-simplified-package-loading].

Therefore, ALE should run ESLint from the project root, when possible,
so that dependencies will load.  This commit does so.

[ESLint RFC 2018-simplified-package-loading]: https://github.com/eslint/rfcs/blob/master/designs/2018-simplified-package-loading/README.md

Fixes: dense-analysis#2787

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

Thanks, I think this should fix the issues people have been seeing.

@w0rp w0rp merged commit 6ad8836 into dense-analysis:master Jan 1, 2020
@w0rp
Copy link
Member

w0rp commented Jan 1, 2020

Cheers! 🍻

@kevinoid
Copy link
Contributor Author

kevinoid commented Jan 1, 2020

If not, feel free to ping me and I'll take another shot at it.

Thanks for reviewing and merging. 🍻

@eliOcs
Copy link

eliOcs commented Apr 5, 2020

Lint is working in my project but fix isn't, after debugging I found that this change should also be included in the eslint fixer for it to properly work in my project.

@kevinoid
Copy link
Contributor Author

kevinoid commented Apr 5, 2020

Thanks for the heads-up @eliOcs. Apparently it was reported in #3094 and #3095, but I created #3096 before realizing. (Which may be useful, since it includes updates to the tests and is ready to merge.)

" Identify project root from presence of node_modules dir.
" Note: If node_modules not present yet, can't load local deps anyway.
let l:modules_dir = ale#path#FindNearestDirectory(a:buffer, 'node_modules')
let l:project_dir = !empty(l:modules_dir) ? fnamemodify(l:modules_dir, ':h:h') : ''

Choose a reason for hiding this comment

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

@kevinoid - for better or worse I'm in a monorepo (using Yarn Workspaces) that has the eslint stuff at the repo root but each "package" has its own package.json and node_modules/ as well (for other dependencies, they just don't have eslint). see my comment here

seems like this line is cd'ing me into the wrong directory. I wonder if we could change the logic to use the modules_dir that has the executable defined as the project dir?

@kevinoid kevinoid deleted the eslint-run-from-project-root branch July 2, 2020 16:25
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.

ESLint 6 couldn't find the plugin
4 participants