[BUG] TSLspImportAll function doesn't work with JavaScript

[LandonSchropp]

FAQ

• I have checked troubleshooting and it didn't resolve my problem.

Issues

• I have checked existing issues and there are no issues with the same problem.

Neovim Version

v0.6.0

Steps to reproduce

1. Install LunarVim using the installation script. (I don’t believe this touches the existing Neovim config, but it’s probably better to have it backed up just in case.)

   INSTALL_SCRIPT="https://raw.githubusercontent.com/lunarvim/lunarvim/rolling/utils/installer/install.sh"
   (yes y | LV_BRANCH=rolling bash <(curl -s "$INSTALL_SCRIPT")) || true

2. Open ~/.config/lvim/config.lua and replace it with the following config.

   lvim.plugins = {
     { "jose-elias-alvarez/nvim-lsp-ts-utils" },
   }
   
   lvim.lsp.on_attach_callback = function(client)
     print("on_attach_callback")
   
     if client.name ~= "tsserver" then
       return
     end
   
     local ts_utils = require("nvim-lsp-ts-utils")
   
     ts_utils.setup({
       auto_inlay_hints = false,
       update_imports_on_move = true,
     })
   
     ts_utils.setup_client(client)
   end

3. Open up LunarVim (lvim) and run :PackerSync. Then close LunarVim. (This step might not be necessary, but I'm adding it just in case.)

4. Open up an existing JavaScript/TypeScript project and delete an import from a file.

5. Run :TSLspImportAll. No files will be imported. A message will be printed that says No code actions available.

Expected behavior

The deleted function show be re-added.

Actual behavior

The deleted import is not re-added.

Debug log

No response

Help

Yes

Implementation help

I would definitely need some guidance on how to fix the problem, but I'm happy to help out.

[LandonSchropp]

Please let me know if the repro steps are insufficient. I can create an example JavaScript repo if necessary.

[jose-elias-alvarez]

Thanks! Installing LunarVim was way easier than I imagined.

I tried this out on an existing TypeScript project and the command works as expected. Are you able to reproduce this without LunarVim, too? If you could put together an example repo, it would be a big help.

[LandonSchropp]

The LunarVim folks have done a great job making the process easy. They're approaching a 1.0 release, so I think they're really trying to make things simple. 🙂

Sure, I'll see if I can get something put together as an example. Maybe there's a weird interaction with my project config or TypeScript version? I'll debug and see what I come up with.

Thanks again for the help!

[jose-elias-alvarez]

Sounds good! TypeScript version is my top suspect. For reference, I'm using 4.5.4.

[LandonSchropp]

I tried uninstalling my local tsserver, and instead, I used the installation through nvim-lsp-config. Unfortunately, I'm seeing the same result. I'm still working on the repro repo, but I thought I'd mention this.

[LandonSchropp]

Okay, I've created an example repo.

I tried the command with both TypeScript and Javascript. Typescript seems to be working great, but JavaScript isn't working.

[jose-elias-alvarez]

Okay! I think I've got it. Basically, the way :TSLspImportAll works is to get diagnostics from the current buffer, filter to keep only diagnostics related to missing imports, then get and run code actions to add all missing imports. By default, TypeScript doesn't check JavaScript files, so no diagnostics are emitted.

By telling the TypeScript compiler to check JavaScript files with the following tsconfig.json, the function works:

{
  "compilerOptions": {
    "target": "es2016",
    "module": "commonjs",
    "allowJs": true,
    "checkJs": true
  }
}

Of course, modifying tsconfig.json may be a no-go on work projects, but there's nothing we can without the diagnostics.

Edit: I'll add a mention of this to the documentation. The "correct" solution is to support this upstream, and I see that typescript-language-server actually added support in typescript-language-server/typescript-language-server#318. I'll test this out to see how it compares to our solution, though I'll mention that ours has a few additional features that make it useful.

[LandonSchropp]

Awesome! Thank you for digging into this. 🙂 I'll give this a shot tomorrow to confirm that this solves the issue for me.

[LandonSchropp]

That did the trick! Thanks again for all the help.

[LandonSchropp]

Hmm, well there is a big downside to the jsconfig.json solution. The checkJs parameter seems to be required. However, enabling it annoying starts checking JavaScript files for TypeScript type declarations.

It would be awesome if this could be handled with the upstream solution to avoid this issue. 🙂

[jose-elias-alvarez]

I read through the documentation for these source actions but wasn't able to get them to work with Neovim, most likely because they're meant to run on save. I also can't say for sure whether they'd work for JavaScript, either.

In your situation, I'd try to find more information about how to run these actions in Neovim and try to set up a solution. Our implementation requires diagnostics and is Neovim-specific but enables a few handy features, like import priority and prompting the user when there are multiple options available.

[LandonSchropp]

I agree your solution sounds better. Also, finding a way to integrate these actions seems a bit beyond my expertise here.

As an alternative, do you know if there's any way to suppress the TypeScript type errors in the Neovim UI? I'd definitely like to leave on the other LSP indicators (such as null-ls), but I don't actually care about the TypeScript-specific errors.

[jose-elias-alvarez]

This suppresses the display of diagnostics in the main window (signs, virtual text, and highlights) but will still show up when using Lualine and other consumers of vim.diagnostic.get:

    -- inside on_attach
    if client.name == "tsserver" then
        vim.diagnostic.disable(bufnr, vim.lsp.diagnostic.get_namespace(client.id))
    end

Not sure if there's a better solution than that. Clearing out the diagnostics completely isn't an option, since then :TSLspImportAll won't have access to them and we're right back where we started.

[jose-elias-alvarez]

I've added a note to the documentation about the interaction in 083c918, so I think that's all we can do for now. When I can, I'll look into the source code actions again and see how we can run them (or how to add support in Neovim if they're not yet supported, which is what I suspect). Thanks for your patience and detailed reports!

[LandonSchropp]

This suppresses the display of diagnostics in the main window...

This is a pretty decent workaround. It's a bummer that it doesn't work everywhere, but it's better than nothing.

The main downside of using checkJs is I'm not sure it's going to work for team projects in the future (although it's fine for me now).

When I can, I'll look into the source code actions again and see how we can run them (or how to add support in Neovim if they're not yet supported, which is what I suspect).

That'd be awesome. It'll be cool to get this working at some point in the future. Would it be possible to request a ticket to track this so I can subscribe for updates?

Thanks for your patience and detailed reports!

Thank you for all the time and energy, as well as for creating several awesome tools I use every day. 🙂

[jose-elias-alvarez]

Thanks for the kind words! I opened #100 to track progress on the new actions.

[LandonSchropp]

Cool, thanks!