tsserver plugin receives old proxy on configuration change

[vemoo]

🔎 Search Terms

tsserver plugin old proxy

🕗 Version & Regression Information

Happens on 4.4.3, 5.2.2 and 5.3.0-dev.20230930 at least.

⏯ Playground Link

No response

💻 Code

Using this https://github.com/vemoo/TypeScript-TSServer-Plugin-Template

npm ci
tsc -p .
cd example
npm ci
code .

Open index.ts, ensure you are using typescript from workspace, "Command Palette..." > "TypeScript: Select TypeScript Version..." > "Use Workspace Version".

"Command Palette..." > "TypeScript: Open TS Server log" (enable it if prompted)

Go back to index.ts and trigger completions at any point. Search log file for my-plugin: getCompletionsAtPosition, a line like this will appear:

Info 151  [12:18:49.536] my-plugin: getCompletionsAtPosition ({"init":1,"create":1})

In the example folder in a terminal run touch tsconfig.json, go back to index.ts and trigger completions. Open log and search for my-plugin: getCompletionsAtPosition again. New lines appear:

Info 244  [12:22:23.378] my-plugin: getCompletionsAtPosition ({"init":2,"create":2})
Info 245  [12:22:23.378] my-plugin: getCompletionsAtPosition ({"init":1,"create":1})

🙁 Actual behavior

When tsconfig.json changes, the plugins init and create are called again but the info.languageService passed here https://github.com/vemoo/TypeScript-TSServer-Plugin-Template/blob/7ef3052284f644c9d15a048f43dcffc07c072936/src/index.ts#L7 is the one the plugin returned on the previous execution, so the language service returned by the plugin ends up doubly decorated.

🙂 Expected behavior

When plugin's create is called, info.languageService should contain an fresh object that doesn't have the decorations from the previous execution.

Additional information about the issue

No response

[sheetalkamat]

This is intended. We do not check if plugin has patched LS instance and whatever LS instance we have will be passed around. This is to ensure that if there are multiple plugins, they can still handle the patching correctly.
Plugin should handle whether they need to patch LS instance or just data associated with it.

[vemoo]

There's no need to check if the LS has been patched, it's sufficient to keep the original LS and use that to create a new LS to pass to plugins on each reload.

I have a fix here: https://github.com/vemoo/TypeScript/tree/fix-plugin-reload specifically this commit: 9239728

Otherwise in my opinion the documentation here https://github.com/microsoft/TypeScript/wiki/Writing-a-Language-Service-Plugin is incomplete because it suffers from the issue I describe.

[vemoo]

I don't think this is a matter of completing documentation because I think it's afecting most existing plugins.

To clarify what's happening this is some code representing what the patched LS may look like after a plugin is applied:

languageServiceV0 = {...originalLanguageService}
languageServiceV0.getCompletionsAtPosition = (fileName, position, options) => {
  if (someCheck()) {
    return customCompletions;
  }
  return originalLanguageService.getCompletionsAtPosition(fileName, position, options)
}
// this is the LS used by typescript
laguageService = languageServiceV0;

then after a reload, create of the plugin is called again and now the LS will be something like:

originalLanguageService = ...;
languageServiceV0 = {...originalLanguageService}
languageServiceV0.getCompletionsAtPosition = (fileName, position, options) => {
  if (someCheck()) {
    return customCompletions;
  }
  return originalLanguageService.getCompletionsAtPosition(fileName, position, options)
}
languageServiceV1 = {...languageServiceV0}
languageServiceV1.getCompletionsAtPosition = (fileName, position, options) => {
  if (someCheck()) {
    return customCompletions;
  }
  return languageServiceV0.getCompletionsAtPosition(fileName, position, options)
}
// this is the LS used by typescript
laguageService = languageServiceV1;

A new layer of recursion is added after each reload.

[sheetalkamat]

It depends on how you handle the patching of LS .. you can always check if it’s already patched which your example is not doing and hence patching it again

[vemoo]

Ok, I see now that for example the angular plugin is doing this:
https://github.com/angular/angular/blob/ced66d4007784de1ed38d7fcd82fd1c6db9b8436/packages/language-service/src/ts_plugin.ts#L17
where getTypescriptLanguageService is:
https://github.com/angular/angular/blob/ced66d4007784de1ed38d7fcd82fd1c6db9b8436/packages/language-service/src/ts_plugin.ts#L200
and isNgLanguageService is:
https://github.com/angular/angular/blob/ced66d4007784de1ed38d7fcd82fd1c6db9b8436/packages/language-service/api.ts#L64

Which effectively means that getTypescriptLanguageService will always return the LS passed on the first call to create. Which I feel is a bit weird because it assumes the previous languageService is still valid.

I still think there will be existing plugins that are not checking because https://github.com/microsoft/TypeScript/wiki/Writing-a-Language-Service-Plugin and https://github.com/orta/TypeScript-TSServer-Plugin-Template don't check.

I'll close the PR and leave this issue for the documentation.