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

tsserver wrong autoimports with multiline imports #29286

Open
geril2207 opened this issue Jun 11, 2024 · 5 comments
Open

tsserver wrong autoimports with multiline imports #29286

geril2207 opened this issue Jun 11, 2024 · 5 comments
Labels
bug-regression wrong behavior that was introduced in a previous commit (please bisect) has:bisected issue has been tracked to a specific commit lsp

Comments

@geril2207
Copy link

geril2207 commented Jun 11, 2024

Problem

Incorrect comma (,) position when using autoimports, if the imports already exist from the module(multiline formatting).
I made bisect and for me the problem was in 2ce4a4d

So the problem:

2024-06-11.16-59-12.mp4

Could be duplicate of #29284

Steps to reproduce

Sorry, I don't want to figure out, how can I provide it to you :). Maybe somebody from your team has already installed tsserver and has projects with typescript, and can reproduce it.

Expected behavior

Works fine on 6e45cd7 (commit before)

2024-06-11.17-00-29.mp4

Neovim version (nvim -v)

above

Vim (not Nvim) behaves the same?

n/a

Operating system/version

fc 39

Terminal name/version

kitty

$TERM environment variable

xterm-kitty

Installation

build from repo

@geril2207 geril2207 added the bug issues reporting wrong behavior label Jun 11, 2024
@zeertzjq zeertzjq added bug-regression wrong behavior that was introduced in a previous commit (please bisect) lsp has:bisected issue has been tracked to a specific commit and removed bug issues reporting wrong behavior labels Jun 11, 2024
@justinmk
Copy link
Member

@alcolmenar

@alcolmenar
Copy link
Contributor

Yea I was afraid that other LSPs might be providing the textEdits in a way just to make it work with Neovim. However, I wasn't able to reproduce this using vtsls. Which tsserver wrapper are you using?

It's possible that the LSP would need to be updated to account for the fix. The fix was made to match closer to VSCode's implementation here https://github.com/microsoft/vscode-languageserver-node/blob/main/textDocument/src/main.ts#L371

In regards to #29284 though, it appears unrelated since that seems like more of an issue with what autocomplete options are provided, but I may be totally wrong there. I'm new to the codebase

@geril2207
Copy link
Author

geril2207 commented Jun 14, 2024

Could you try the following? Don't forget to use latest nvim dev.

  1. Create /tmp/repro folder. If you want to change this folder, you also need to change root path in repro.lua as well.
  2. Create /tmp/repro/repro.lua with the following content:
local root = vim.fn.fnamemodify("/tmp/repro/.repro", ":p") -- it will download modules in /tmp/repro/.repro

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
	vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
	vim.fn.system({
		"git",
		"clone",
		"--filter=blob:none",
		"--single-branch",
		"https://github.com/folke/lazy.nvim.git",
		lazypath,
	})
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
	{
		"neovim/nvim-lspconfig",
		config = function()
			local on_attach = function(client, bufnr)
				vim.lsp.completion.enable(true, client.id, bufnr, { autotrigger = false })
			end

			local lspconfig = require("lspconfig")
			local capabilities = require("cmp_nvim_lsp").default_capabilities()

			lspconfig.tsserver.setup({
				on_attach = on_attach,
				capabilities = capabilities,
			})
		end,
	},
	{
		"hrsh7th/nvim-cmp",
		dependencies = {
			"hrsh7th/cmp-nvim-lsp",
		},
		config = function()
			local cmp = require("cmp")

			cmp.setup({
				mapping = cmp.mapping.preset.insert({
					["<C-b>"] = cmp.mapping.scroll_docs(-4),
					["<C-f>"] = cmp.mapping.scroll_docs(4),
					["<C-Space>"] = cmp.mapping.complete(),
					["<C-e>"] = cmp.mapping.abort(),
					["<CR>"] = cmp.mapping.confirm({ select = true }), -- Accept currently selected item. Set `select` to `false` to only confirm explicitly selected items.
				}),
				sources = cmp.config.sources({
					{ name = "nvim_lsp" },
				}, {
					{ name = "buffer" },
				}),
			})
		end,
	},
}

require("lazy").setup(plugins, {
	root = root .. "/plugins",
})
  1. npm install -g typescript typescript-language-server
  2. mkdir /tmp/repro-test
  3. cd /tmp/repro-test
  4. npm install @types/node -D
  5. Create /tmp/repro-test/main.ts file with the following content:
import {
  sep,
  join,
  parse,
  posix,
  win32,
  format,
  dirname,
  extname,
  resolve,
  basename,
} from "path";
  1. nvim . -nu /tmp/repro/repro.lua
  2. Open main.ts. Try to auto import normalize with completion.
2024-06-14.11-18-13.mp4

Also you can try vtsls, for me they have the same issue.

@iurimateus
Copy link

iurimateus commented Jun 14, 2024

See https://github.com/pmizio/typescript-tools.nvim/blob/master/lua/typescript-tools/protocol/text_document/did_change.lua#L11

Also, vscode's typescript server doesn't comply with the spec (not sure what the state of tsserver/vtsls is; if vtsls just bundles the vscode package, I expect it to need the same patch as above).

@geril2207
Copy link
Author

geril2207 commented Jun 14, 2024

I expect it to need the same patch as above).

Do you mean patch from typescript-tools? If so, typescript-tools have the same issue

@justinmk justinmk changed the title Nightly release: tsserver wrong autoimports with multiline imports tsserver wrong autoimports with multiline imports Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-regression wrong behavior that was introduced in a previous commit (please bisect) has:bisected issue has been tracked to a specific commit lsp
Projects
None yet
Development

No branches or pull requests

5 participants