jose-elias-alvarez / nvim-lsp-ts-utils

Utilities to improve the TypeScript development experience for Neovim's built-in LSP client.
The Unlicense
438 stars 18 forks source link

[BUG] Stopped working correctly with local ESLint #66

Closed VVKot closed 3 years ago

VVKot commented 3 years ago

FAQ

Issues

Neovim Version

NVIM v0.6.0-dev Build type: RelWithDebInfo LuaJIT 2.1.0-beta3 Compilation: /usr/bin/cc -g -O2 -fdebug-prefix-map=/build/neovim-cxwmmX/neovim-0.6.0~ubuntu1+git202109061203-3f526feeb-adeb5640f=. -fstack-protector-strong -Wformat -Werror=format-security -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -DNVIM_TS_HAS_SET_MATCH_LIMIT -O2 -g -Og -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion -Wmissing-prototypes -Wimplicit-fallthrough -Wvla -fstack-protector-strong -fno-common -fdiagnostics-color=auto -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -DMIN_LOG_LEVEL=3 -I/build/neovim-cxwmmX/neovim-0.6.0~ubuntu1+git202109061203-3f526feeb-adeb5640f/build/config -I/build/neovim-cxwmmX/neovim-0.6.0~ubuntu1+git202109061203-3f526feeb-adeb5640f/src -I/build/neovim-cxwmmX/neovim-0.6.0~ubuntu1+git202109061203-3f526feeb-adeb5640f/.deps/usr/include -I/usr/include -I/build/neovim-cxwmmX/neovim-0.6.0~ubuntu1+git202109061203-3f526feeb-adeb5640f/build/src/nvim/auto -I/build/neovim-cxwmmX/neovim-0.6.0~ubuntu1+git202109061203-3f526feeb-adeb5640f/build/include Compiled by buildd@lgw01-amd64-034

Features: +acl +iconv +tui See ":help feature-compile"

system vimrc file: "$VIM/sysinit.vim" fall-back for $VIM: "/usr/share/nvim"

Run :checkhealth for more info

Steps to reproduce

Have ESLint in node_modules/.bin/eslint, no globally installed one. Plugin does not work correct. More details in #65.

Answering the specific questions @jose-elias-alvarez:

  1. lua print(require("nvim-lsp-ts-utils.utils").buffer.root()) returns "nil"
  2. lua print(require("nvim-lsp-ts-utils.utils").resolve_bin("eslint")) returns "using local executable node_modules/.bin/eslint. node_modules/.bin/eslint"

Hope that helps

Expected behavior

Works with local ESLint

Actual behavior

Only works with globally installed ESLint (i.e. available on the $PATH)

Debug log

No response

Help

Yes, but I don't know how to start. I would need guidance

Implementation help

No response

jose-elias-alvarez commented 3 years ago

Thanks for the info! Could you also post your config for this plugin?

VVKot commented 3 years ago

Sure @jose-elias-alvarez , here you go: https://github.com/VVKot/dotfiles/blob/0ce1a23a1c03c9019c3078101a5c6e1003b9b18a/config/nvim/after/plugin/lsp.lua#L252-L266

jose-elias-alvarez commented 3 years ago

Thanks! Nothing immediately jumps out at me as a potential issue, but it’s definitely unusual that buffer.root is returning nil. We are using this util from nvim-lspconfig to resolve the root from the current buffer’s path using tsconfig.json and package.json as markers, with .git as a fallback.

Is there something unusual about the project’s structure in this case? If you can, it would be a big help if you could play around with the nvim-lspconfig method and see if you can get it to return the correct root. We could them see how to update root detection here.

I’m also planning on updating null-ls to allow returning a command from a callback, which will let you modify the command based on the current directory, so if we can’t figure this out, that’ll provide a solution.

VVKot commented 3 years ago

@jose-elias-alvarez I don't think there is anything specific to the layout that I have, and it was 100% working until ~last two weeks. I can try to do a git bisect for nvim/lspconfig/ts-utils/null-ls to see if I can find a specific commit.

roginfarrer commented 3 years ago

Sorry if this is adding noise, but I'm also experiencing the same issue. Both eslint_d and eslint stopped working. eslint_d silently fails (no error), but also produces no lints where they should. When I use eslint (which I don't have globally installed, just in the project), I get this error:

Error executing vim.schedule lua callback: ...ck/packer/start/plenary.nvim/lua/plenary/async/async.lua:14: The coroutine failed with this message: ...e/pack/packer/start/null-ls.nvim/lua/null-ls/helpers.lua:156: command eslint is not executable (make sure it's installed and on your $PATH)

roginfarrer commented 3 years ago

Still need to test further, but I updated my config to use eslint_opts.condition, and now eslint is running correctly:

ts_utils.setup({
  eslint_bin = 'eslint_d',
  eslint_enable_diagnostics = true,
+ eslint_opts = {
+    condition = function(utils)
+      return utils.root_has_file('.eslintrc.js')
+    end,
+    diagnostics_format = '#{m} [#{c}]',
+  }
})
jose-elias-alvarez commented 3 years ago

I haven't had the time to look into this too deeply but am still unable to replicate it. I put together this repository, which will contain a local copy of eslint after cloning it and running npm install. Both code actions and diagnostics work when using the following minimal Neovim config and opening test.js:

local lspconfig = require("lspconfig")

lspconfig["tsserver"].setup({
    on_attach = function()
        require("nvim-lsp-ts-utils").setup({
            eslint_enable_diagnostics = true,
            debug = true,
        })
    end,
})

require("null-ls").config({ debug = true })
lspconfig["null-ls"].setup({})

I can also confirm from both the debug output in :messages and the null-ls debug log that it's correctly using the local executable (and I even tried uninstalling the global executable just to be sure).

From the information here, I assume the issue has to do with resolving the project root when using more complex project structures, but since the projects I work on are relatively simple and I don't have a lot of bandwidth to investigate, I'll either need a full-on reproduction or someone adventurous to dig through the code and find out what's going on.

roginfarrer commented 3 years ago

I think you're right, this is probably due to something going haywire with the ESLint dependencies in the codebase. This is specifically happening to me in a large monorepo. I was also able recreate the issue when running eslint_d in the terminal, outside of nvim.

I also just booted up a smaller codebase, and I'm not having the issue there.

Oh, and the update I made about changing the config to use eslint_opts.condition was actually not useful. I think that change was just correlated with the eslint working again.

jose-elias-alvarez commented 3 years ago

Could anyone who's having this issue try #77 and see if that makes a difference?

VVKot commented 3 years ago

@jose-elias-alvarez sadly it does not help but I found a way to repro it! As @roginfarrer correctly pointed out, it is a monorepo/setup-specific problem: https://github.com/jose-elias-alvarez/local-eslint-test/pull/1

jose-elias-alvarez commented 3 years ago

Thanks, I think I see the issue, but I've gone back quite a few commits and it seems that the same issue existed (if it worked, it was probably a coincidence and wouldn't have worked if eslint wasn't installed at the project's root).

The best solution I can think of right now to handle the case in your PR is to traverse the file's parents and check if node_modules exists and contains an eslint executable. I just pushed a commit to #77 that does this, so could you give it a shot?

Edit: And also let me know if you have any other ideas for solving this.

VVKot commented 3 years ago

Sweet, the new version of #77 works! 🎉 IMO I'd try to keep it as close to ESLint LSP (https://github.com/neovim/nvim-lspconfig/pull/1273) as possible (and probably worth trying to switch over some functionality to it?)

roginfarrer commented 3 years ago

I'm still having some issues when running #77. I'm not getting any eslint errors. That said, I'm not sure if that's the fault of this plugin.

Context:

I'm working in a large monorepo (~20) packages. Each package has an .eslintrc.js. We normally have a custom yarn lint command that handles resolving the location of the eslint executable, and this command works well (i.e., I get the expected output).

However, if I just try to run eslint directly, I get no output (which I expect is why this plugin isn't providing the diagnostics I'd expect).

# Running from the root of the monorepo doesn't work
yarn workspace @workspace/pkg eslint packages/pkg

# Running when the cwd is the package also doesn't work
yarn eslint .

Note that yarn is using the ESLint executable is in the root dir's node_modules:

node_modules/
  eslint/
    bin/
packages/
  package/
    package.json
    src/
      file-that-should-lint.js
package.json
yarn.lock

Somehow, the new ESLint LSP added to nvim-lspconfig somehow still works correctly 🤔

jose-elias-alvarez commented 3 years ago

Thanks to both of you for checking!

We could theoretically resolve ESLint config files using a similar approach of traversing parent directories and pass the config path directly via the CLI, but since there are so many possible config file formats and other project structures, I'm not sure it's worth it, especially given that the ESLint LSP seems to handle cases like this. I don't know exactly what the language server is doing under the hood, but the Node API is fundamentally far more powerful than the CLI, so I think we're reaching the limits of out-of-the-box functionality here.

My plan is to merge the PR and add a note to the documentation to suggest using the language server in cases too complicated for eslint $FILE.