LSP not activating in Deno Node Compatible projects

[VioletWoodETL]

Description

Neovim version: v0.11.5

Noticed in downstream @ LazyVim/LazyVim#6816

It is common, for example in a Tauri project running Deno, to have a Deno project with a package.json and a deno.lock without a deno.json. See https://docs.deno.com/runtime/fundamentals/node/#first-class-package.json-support. However, after this change, a Deno project set up this way will no longer have LSP support.

Reproduction script using lazy.nvim repro tools. This creates a typical LazyVim setup that has vtsls and denols enabled:

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
  spec = {
    { "LazyVim/LazyVim", import = "lazyvim.plugins" },
    { "LazyVim/LazyVim", import = "lazyvim.plugins.extras.lang.typescript" },
    {
      "neovim/nvim-lspconfig",
      commit = "b34fbdf",
      opts = {
        servers = {
          -- as an example, this is also applicable for other non-Deno LSPs that technically work.
          vtsls = {
            enabled = true,
          },
          denols = {
            enabled = true,
          },
        },
      },
    },
  },
})

Running this and trying to edit Typescript or Javascript in a repo with package.json and deno.lock, without a deno.json, will result in no LSPs being triggered.

An open question here to address this regression is which LSP should be activated in this situation?

[justinmk]

cc @xbwwj

after this change, a Deno project set up this way will no longer have LSP support.

good lord we are going in circles.

to address this regression is which LSP should be activated in this situation?

the general answer is that the config should target the common case, and document how to change things for the uncommon case.

Nvim can't magically do the right thing for every weird project.

i have no idea what the most common setup is, that is up to users who actually use this stuff.

[justinmk]

Just merged #4207 , does it help at all?

[AsPulse]

I am the author of #4207, and I don't think this PR resolves this issue.
My implementation treats projects as non-Deno if a package.json file exists.

I'll try to create a fix.
Theoretically, we should start by treating it as a Deno project without question only if deno.lock exists, and then differentiate its priority from other root markers; this should be possible.

@VioletWoodETL It would be very helpful if you could provide a sample of a Tauri project running on Deno in a public repository or something similar!

[AsPulse]

@justinmk I believe the following PR will resolve this issue. (I apologize for it being a bit large change.)

• feat(lsp): use smarter detection of typescript project #4217

Since implementations that separate Deno and non-Deno projects exist in various places, I've consolidated them into one location and introduced more robust judgment than the previous codebase.

I'd appreciate it if you could review it!

@VioletWoodETL
You can try the fixes I'm proposing on your end by doing the following.
If you have time, it would be very helpful if you could let me know whether it worked or not.

  {
    'AsPulse/nvim-lspconfig',
    branch = 'feat(lsp)/smarter-typescript-project-detection',
    ...

[VioletWoodETL]

@AsPulse Here's the example repo you asked for: https://github.com/VioletWoodETL/example-deno-tauri

I tried changing the repro script to this and feeding it to nvim -u:

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
  spec = {
    { "LazyVim/LazyVim", import = "lazyvim.plugins" },
    { "LazyVim/LazyVim", import = "lazyvim.plugins.extras.lang.typescript" },
    {
      "AsPulse/nvim-lspconfig",
      branch = "feat(lsp)/smarter-typescript-project-detection",
      opts = {
        servers = {
          -- as an example, this is also applicable for other non-Deno LSPs that technically work.
          vtsls = {
            enabled = true,
          },
          denols = {
            enabled = true,
          },
        },
      },
    },
  },
})

However, I did not get any results with the example repo, :LspInfo is still dry on active clients. That being said, that's probably LazyVim's fault, the config is highly complex. You may have more luck accurately reproducing results on your end with that Tauri sample.

[eriklindebratt]

I get the feeling that it's too generic to detect Deno/non-Deno for a buffer by only checking for package.json, deno.json, deno.lock files etc. being present.

Would it not be better to try to make use of the Deno LSP server's settings deno.enablePaths (or deno.enable) setting? That allows for enabling/disabling denols per-path, or disabling it completely using deno.enable.

[AsPulse]

@VioletWoodETL Thank you for the repository.
In my environment, upon opening src/main.ts from that repository, I confirmed that denols was activated (and no other LSP was activated).

The reason it's not working in your environment is likely because you are using a deprecated configuration method (specifying something in lazy.nvim's opts passes it to the require('lspconfig').setup function, which is an old method).

While we might need to introduce the same changes in the old config method, I'm unsure if it's the repository's policy to actively support old configurations.

but for now, using the supported method below should work:

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
  spec = {
    { "LazyVim/LazyVim", import = "lazyvim.plugins" },
    { "LazyVim/LazyVim", import = "lazyvim.plugins.extras.lang.typescript" },
    {
      "AsPulse/nvim-lspconfig",
      branch = "feat(lsp)/smarter-typescript-project-detection",
      init = function() 
        vim.lsp.enable('denols')
        vim.lsp.enable('vtsls')
      end,
    },
  },
})

[dpetka2001]

The reason it's not working in your environment is likely because you are using a deprecated configuration method (specifying something in lazy.nvim's opts passes it to the require('lspconfig').setup function, which is an old method).

That is not accurate. opts being present does not necessarily mean that require("lspconfig").setup() will be called. That's because later on in the plugin spec there's a config function that overwrites the default function that would have been called under normal circumstances. No require("lspconfig").setup() is being called. Instead mason-lspconfig is being used to set up automatically the servers, which has transitioned to the new vim.lsp.config/vim.lsp.enable features. nvim-lspconfig is just used as a placeholder to load mason-lspconfig among other things.

According to OP from the LazyVim issue

I don't expect denols to get attached, per se. I expect vtsls to get attached, as that LSP was triggering before for my deno/node projects and worked perfectly fine.

The new branch from

"AsPulse/nvim-lspconfig",
branch = "feat(lsp)/smarter-typescript-project-detection",

seems to not solve this issue. vtsls should have been attached when opening src/main.ts file according to OP past comments, not denols. commit = "b34fbdf" correctly attaches vtsls server in that project when opening src/main.ts file. According to OP deno is only used as a package manager in that project

such as a fresh Tauri project with deno selected as a package manager, and deno can function perfectly fine as the package manager without deno.json if there exists a package.json and a deno.lock

and vtsls should be the LSP server attached in src/main.ts file.

I don't know what the correct solution should be and I agree that nvim-lspconfig should only cover a general case and users should change root_dir according to their project's requirements. Maybe they could also use :h exrc for local project configurations?

PS: I don't know which LSP server should attach in this case, as I'm not familiar at all with Deno projects. If vtsls should attach, then I think the logic is still wrong (according to what OP said at least). If denols should attach in that Tauri project, then indeed a change is needed in LazyVim and then it will work properly (I tested with branch = "feat(lsp)/smarter-typescript-project-detection" and it works correctly if denols should attach). I just said about vtsls attaching because that is what OP mentioned himself and assumed that is what he wants, but I don't know what the correct answer is with regards to which LSP server should attach in a project like VioletWoodETL/example-deno-tauri.

[AsPulse]

I made the assumption that denols should be attached when both package.json and deno.lock exist (my apologies), but since it seems debatable, I've reverted the PR to Draft.
Thank you for your feedback.

But attaching denols in that situation was an explicit configuration, so, it's very easy to change the logic to attach non-Deno LSPs like vtsls or ts_ls in such situations.

I agree with the opinion that users should individually handle configurations not covered by nvim-lspconfig. However, given the widespread use of nvim-lspconfig, I also believe it's good for user to cover as many cases as we can.

[AsPulse]

I have prepared two branches for verification purposes or as a workaround for those currently facing this issue.

In the situation that both package.json and deno.lock exist,

• Attach denols

    "AsPulse/nvim-lspconfig",
    branch = "feat(lsp)/smarter-typescript-project-detection-deno",

• Attach non-Deno LSP (vtsls, ts_ls, tsgo...)

    "AsPulse/nvim-lspconfig",
    branch = "feat(lsp)/smarter-typescript-project-detection-node",

I hope they are useful.

[jceb]

I also ran into this issue and I'm wondering why package.json needs to be included in the list of added by #4207. Deno supports package.json and I'd leave it to the user to decide whether this lsp shall be activated. package.json is standard Deno nowadays and should definitely not be part of the list.

Heuristics on detecting whether to run Deno can only go so far. My preference is to leave it to the user and we make the list of files configurable that prevents denols from being attached. For example, I'm having a hard time with the current configuration and a project that uses pnpm for storybook (currently not supported by Deno) and Deno for executing the application.

[eriklindebratt]

@jceb I agree.

This is how I have handled denols/vtsls conflicts. It basically checks if the current file is whitelisted in deno.enablePaths and if so it sets root_dir to nil for vtsls (effectively ensuring only Deno LSP server is active for Deno files). Does the job but can probably be improved.

This means that the Deno LSP server is actually attached to non-Deno TypeScript files, but since the server checks deno.enablePaths (and deno.enable) setting it won't do anything on files not matching those settings.

Ideally I'd like deno.enablePaths to be respected here in lspconfig.

Current approach of checking for package.json/deno.json(c)/deno.lock basically makes it impossible to mix Deno and non-Deno files within the same project (unless I'm missing something), so I've locked lspconfig to a specific commit.

[AsPulse]

I also agree with the opinion that "there is no perfect heuristic for determining which LSP to fully attach, so it should be customizable for users." However, that has been possible from the beginning. It might be a bit tedious, but root_dir and on_attach were fully customizable.

Therefore, from my standpoint, what nvim-lspconfig should do is create "better heuristics that cover as many common cases as possible." I think there will also be opposition to making heuristics more complex. Such things often make debugging difficult or confuse users with unintended behavior. How to handle these should be discussed, but whether we make the heuristics simple or complex, the behavior regarding LSP attachment is still completely up to the user. It's not just "which LSP to attach to which files" that's customizable, but "the algorithm for determining which LSP to attach to which files," which is very Neovim-like behavior, and I believe that's already achieved.

In other words, referring to lsp's deno.enablePaths for determining isn't adding any new customizability; it's just an improvement to the heuristics. It's not a silver bullet, and like previous changes, it will cause unintended results for some people. I don't think this can be justified solely on the grounds that "heuristic can't detect everything, so leave it to user".

---

Ultimately, explicity is necessary to escape heuristics.
This means providing options for the issues discussed so far, such as "how to handle the coexistence of package.json and deno.lock" and "how to handle deno.enablePaths."

However, I don't think this falls within the scope of nvim-lspconfig.

Looking at the series of discussions, I think the most organized solution would be to prepare a separate plugin that allows setting convenient options as mentioned above, and provides on_attach and root_dir. (I will try to create it if I have time.)

[jceb]

@AsPulse this sounds like a good solution. In the meantime, I suggest that the currently hard-coded non_deno_path configuration of PR #4207 is reverted to restore the original behavior.