Support configuring TypeScript plugins#61756
Conversation
|
@mjbvz for review please |
mjbvz
left a comment
mjbvz
left a comment
There was a problem hiding this comment.
Looks good! Just a few changes.
Let's try to get this in this week because we go into a stabilization period for VS Code 1.29 next week
| } | ||
| } | ||
|
|
||
| public configurePlugin(pluginName: string, configuration: any, reconfigureOnRestart?: boolean): any { |
There was a problem hiding this comment.
Scope this to TS 3.2:
if (this._apiVersion.gte(API.v320)) {
...
}Not sure if it should also cause the command to fail. Do you need this information on your side?
| ) { } | ||
|
|
||
| public execute(pluginName: string, configuration: any) { | ||
| this.lazyClientHost.value.serviceClient.configurePlugin(pluginName, configuration, true /* reconfigureOnRestart */); |
There was a problem hiding this comment.
Accessing lazyClientHost.value will force the typescript server to be created which can be expensive. Let's try to defer this instead so that if the server has not been activated, this command just stores received config values until activation takes place.
There was a problem hiding this comment.
That makes sense. Though I'm stuck on where I would insert that code which sends the configuration command upon server start. For example, to listen to onTsServerStarted, I need access to the TypeScriptServiceClient object. Which starts the server as soon as it's constructed. Any ideas?
There was a problem hiding this comment.
One option: have a PluginConfigurationProvider class that stores the plugin config map. Then both the command and the service client share this object.
The class interface could be something like:
class PluginConfigurationProvider {
set(name: string, config: any): void;
readonly entries: Iterator<{ plugin: string, config: any }>;
readonly onChangedConfiguration: vscode.Event<{ plugin: string, config: any }>;
} There was a problem hiding this comment.
OK, so what's a reliable way to check whether the service is activated yet so I know whether to send the command yet? Check lazyClientHost.hasValue?
There was a problem hiding this comment.
Yes. I'd try to avoid accessing the host entirely though and just go through the PluginConfigurationProvider object. Then the service can listen to the onChangedConfiguration to pick up changes that are broadcast by the implementation of set
Follow up on #61756 Two fixes: - Avoid allowing the `_typescript.configurePlugin` to activate the ts extension non-lazily by instead using a `PluginConfigProvider` - Restrict configurePlugin to TS 3.1.4
2 participants
Depends on microsoft/TypeScript#28106
configurePluginstsserver command (see configurePlugins command for tsserver TypeScript#28106)configurePluginscall.