Auto-import duplication because of redundant additionalTextEdits sent by LSP in Zed

[Blankeos]

Describe the bug

Whenever I try to import anything from Svelte, the import ... from the autoimport gets duplicated.

More details on the issue at Zed's repo: zed-industries/zed#19214 (comment)

(Video by @ArturGalstyan1)

Screen.Recording.2024-10-15.at.12.28.45.mov

I think there's two problems here:

1. Zed's lack of safeguards against processing redundant additionalTextEdits.
2. Svelte's LSP sending redundant additionalTextEdits.

Wondering if the Svelte folks know if this is an issue worth fixing or if Zed just needs to tweak their request.

This issue does not exist in VSCode by the way. I compared the logs and Zed just sends additionalTextData when it selects an import under the dropdown.

Reproduction

1. Install Zed
2. Install the Svelte Extension in Zed (Cmd + Shift + X)
3. Go to any .svelte file.
4. Write any import like page, then ctrl + space to get.

You can also debug the language server by opening the command palette (Cmd + Shift + P), then clicking on RPC Messages under svelte-language-server:

Expected behaviour

No duplicate imports

System Info

• OS: macOS 15.1.1
• IDE: Zed: v0.164.2 (Zed)
• I unfortunately can't find the Language Server version. I assume latest as of now. This is the code that the Zed Svelte Extension uses to initialize the LSP.

Which package is the issue about?

svelte-language-server

Additional Information, eg. Screenshots

If this is just expected behavior from the LSP, feel free to close! Thank you!

[jasonlyu123]

Can you check if Zed sent the resolve request twice? I can't think of another possibility where this might happen. We only have two places where we set additionalTextEdits. One is to split part of TextEdit to additionalTextEdits but I didn't see textEdit in your log.

language-tools/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts

Lines 874 to 885 in 02db54d

	completionItem.additionalTextEdits = [
	TextEdit.replace(
	{
	start,
	end: {
	line: start.line,
	character: wordRange.start.character
	}
	},
	newText.substring(0, wordRange.start.character - start.character)
	)
	];

The other is to add the edit either to an empty array or existing if any already exists. But since there isn't any other place where we set it, unless the same code ran twice it should always be the former. If this is the case, We could remove the redundant concat but you should still ask why Zed sent the request twice since it's just a waste of time.

language-tools/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts

Lines 983 to 985 in 02db54d

	completionItem.additionalTextEdits = (completionItem.additionalTextEdits ?? []).concat(
	edit
	);

[Blankeos]

Thanks for getting back! @jasonlyu123. So yeah I think you're 100% right. Thanks for narrowing this down! It is definitely the "Zed sends the request twice" case. I wasn't able to catch it the first time because I assumed that the ctrl + space hotkey only gets suggestions. Not resolve completions. It in-fact resolves completions for every single item in the suggestions.

I honestly don't know why though.

zed-bug-svelte.mp4

[SomeoneToIgnore]

Thank you for the discussion.
From Zed's side, this is indeed a bug and zed-industries/zed#22448 fixes it, in the next release there will be one resolve request per item only.

No strong opinions, but I find this behavior confusing in general, as no other servers (vtsls, rust-analyzer, pyright, etc.) are doing nothing similar.

I see a small hint to this in https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#completionItem

/**
 * A data entry field that is preserved on a completion item between
 * a completion and a completion resolve request.
 */

as something that the language server manages, and can omit in the first resolve response.

[jasonlyu123]

As far as I remember, one of the reasons is that we had additional text edits before resolving to workaround a VSCode behaviour. I can check if it's still needed or if there is another way to solve it.

I just think it still makes sense to ask if it's intentional for Zed to resolve the completion item twice. It's also the first time I saw a client that does this.