Add custom Node option to run TS Server #191019

gabritto · 2023-08-22T20:34:47Z

This PR creates a new setting that is a path to a Node installation that should be used to run TS Server. This allows users of desktop VSCode to run TS Server with a Node version that doesn't have a hard memory limit at 4GB, unlike the bundled version of Node. The custom memory limit can be set through the existing "Max TS Server Memory" setting.

jakebailey · 2023-08-22T20:58:08Z

extensions/typescript-language-features/src/typescriptServiceClient.ts

@@ -173,6 +175,11 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 			this.restartTsServer();
 		}));

+		this._nodeVersionManager = this._register(new NodeVersionManager(this._configuration, context.workspaceState));
+		this._register(this._nodeVersionManager.onDidPickNewVersion(() => {
+			this.restartTsServer();


I'm assuming that it didn't turn out to be possible to block startup on the user accepting the setting?

I couldn't figure out how to do it easily, no. But in any case, the node path setting can change at some point, so we need the code above, I think.

jakebailey · 2023-08-22T20:59:53Z

extensions/typescript-language-features/src/tsServer/nodeManager.ts

+			if (this.configuration.localNodePath) {
+				if (this.useWorkspaceNodeSetting === undefined) {
+					setImmediate(() => {
+						this.promptAndSetWorkspaceNode();
+					});
+				}
+				else if (this.useWorkspaceNodeSetting) {
+					this._currentVersion = this.configuration.localNodePath;
+				}


This code looks the same as the code in the block below; probably could be pulled out into a method.

They're the same except the version below uses this.updateActiveVersion instead of assigning directly as in this._currentVersion = this.configuration.localNodePath (which is kinda special because we're initializing the class at that point), so didn't seem worth it to extract into a method with an option

jakebailey · 2023-08-22T21:00:52Z

extensions/typescript-language-features/src/tsServer/nodeManager.ts

+const useWorkspaceNodeStorageKey = 'typescript.useWorkspaceNode';
+
+export class NodeVersionManager extends Disposable {
+	private _currentVersion: string | undefined;


I assume the "version" moniker is left over from copying the tsdk code? Was confused as to whether or not this was checking versions or something until I realized that this was acutally managing the node path.

Yeah, do you think simply NodeManager would be better?

I was imagining NodePathManager and _currentPath, but I also haven't gone to look for examples that would indicate what's canonical to this repo's style.

mjbvz · 2023-08-23T23:25:46Z

extensions/typescript-language-features/src/tsServer/nodeManager.ts

+
+		const allow = vscode.l10n.t("Allow");
+		const dismiss = vscode.l10n.t("Dismiss");
+		const neverAllow = vscode.l10n.t("Never in this workspace");


Once the user selects never, is there a way for them still opt in later on?

I updated the code to remember the prompt answer for a specific node path. So if you change the workspace node path to point to a different path, the prompt will show up again. However, it's a bit contrived to do that if you select "never" by accident. Should I add a command to use the workspace node path?

gabritto · 2023-08-24T01:13:49Z

extensions/typescript-language-features/src/configuration/configuration.electron.ts

+
+	private findNodePath(): string | null {
+		try {
+			const out = child_process.execFileSync('node', ['-e', 'console.log(process.execPath)'], {


Context: I tried doing the node detection using which, like other language extensions do, but it does not work if you have a node version manager like volta, because if you do, your path points to volta's node.exe wrapper, and if you use that to run TS Server, it crashes.
This alternative approach was suggested by @jakebailey.

Is there a reason to not do this for all paths?

What do you mean for all paths?

gabritto · 2023-08-28T18:43:44Z

@mjbvz updates on this?

Titozzz · 2023-09-19T09:26:03Z

When trying to use this the TSServer instantly crashes with no log, I suspect a permission error maybe? Any idea ? @gabritto

2023-09-19 11:22:46.128 [error] TSServer exited. Code: 1. Signal: null
2023-09-19 11:22:46.128 [info] TSServer log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-EvKNgY/tsserver.log undefined
2023-09-19 11:22:46.128 [info] Starting TS Server undefined
2023-09-19 11:22:46.128 [info] Using tsserver from: /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js undefined
2023-09-19 11:22:46.128 [info] Using Node installation from /usr/local/bin/node to run TS Server undefined
2023-09-19 11:22:46.128 [info] <syntax> Log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-0QawBe/tsserver.log
2023-09-19 11:22:46.128 [info] <syntax> Forking...
2023-09-19 11:22:46.129 [info] <syntax> Starting...
2023-09-19 11:22:46.129 [info] <semantic> Log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-Vvnhnw/tsserver.log
2023-09-19 11:22:46.129 [info] <semantic> Forking...
2023-09-19 11:22:46.129 [info] <semantic> Starting...
2023-09-19 11:22:46.164 [error] TSServer exited. Code: 1. Signal: null
2023-09-19 11:22:46.164 [info] TSServer log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-Vvnhnw/tsserver.log undefined
2023-09-19 11:22:46.164 [info] Starting TS Server undefined
2023-09-19 11:22:46.164 [info] Using tsserver from: /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js undefined
2023-09-19 11:22:46.164 [info] Using Node installation from /usr/local/bin/node to run TS Server undefined
2023-09-19 11:22:46.164 [info] <syntax> Log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-OKk9z9/tsserver.log

gabritto · 2023-09-19T16:21:20Z

When trying to use this the TSServer instantly crashes with no log, I suspect a permission error maybe? Any idea ? @gabritto

2023-09-19 11:22:46.128 [error] TSServer exited. Code: 1. Signal: null
2023-09-19 11:22:46.128 [info] TSServer log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-EvKNgY/tsserver.log undefined
2023-09-19 11:22:46.128 [info] Starting TS Server undefined
2023-09-19 11:22:46.128 [info] Using tsserver from: /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js undefined
2023-09-19 11:22:46.128 [info] Using Node installation from /usr/local/bin/node to run TS Server undefined
2023-09-19 11:22:46.128 [info] <syntax> Log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-0QawBe/tsserver.log
2023-09-19 11:22:46.128 [info] <syntax> Forking...
2023-09-19 11:22:46.129 [info] <syntax> Starting...
2023-09-19 11:22:46.129 [info] <semantic> Log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-Vvnhnw/tsserver.log
2023-09-19 11:22:46.129 [info] <semantic> Forking...
2023-09-19 11:22:46.129 [info] <semantic> Starting...
2023-09-19 11:22:46.164 [error] TSServer exited. Code: 1. Signal: null
2023-09-19 11:22:46.164 [info] TSServer log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-Vvnhnw/tsserver.log undefined
2023-09-19 11:22:46.164 [info] Starting TS Server undefined
2023-09-19 11:22:46.164 [info] Using tsserver from: /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js undefined
2023-09-19 11:22:46.164 [info] Using Node installation from /usr/local/bin/node to run TS Server undefined
2023-09-19 11:22:46.164 [info] <syntax> Log file: /Users/thibault/Library/Application Support/Code - Insiders/logs/20230919T112235/window1/exthost/vscode.typescript-language-features/tsserver-log-OKk9z9/tsserver.log

Was the node path /usr/local/bin/node detected by vscode or provided by you?
Also, are you using any node manager? What version of Node are you using?

gabritto · 2023-09-20T02:04:39Z

@Titozzz Can you open an issue about this so we can keep track of it? Thanks!

Titozzz · 2023-09-20T07:14:11Z

I tried using "node" and "/usr/local/bin/node", and "bun" out of curiosity.

I do use n to manage my node versions

gabritto · 2023-09-20T16:49:42Z

I tried using "node" and "/usr/local/bin/node", and "bun" out of curiosity.

I do use n to manage my node versions

And did TSServer crash with both "node" and "/usr/local/bin/node"? Do you know what version of Node was being used when the crash happened?

gabritto added 7 commits August 11, 2023 16:03

Add config for running tsserver on custom node

7de74a6

log when custom node install is used

5f288d7

create node version manager

080e16a

get node path from node version manager everywhere

d32c60e

modify prompt

17a3c23

fix useIpc

f8fcc17

Merge branch 'main' into main

b3830ba

gabritto marked this pull request as ready for review August 22, 2023 20:46

jakebailey reviewed Aug 22, 2023

View reviewed changes

vscodenpa assigned mjbvz Aug 22, 2023

mjbvz added this to the August 2023 milestone Aug 23, 2023

mjbvz reviewed Aug 23, 2023

View reviewed changes

gabritto added 3 commits August 23, 2023 18:01

use spawn for custom node and set windowsHide

8210f67

detect node

ac96741

Merge branch 'main' into main

5d9c2bf

gabritto commented Aug 24, 2023

View reviewed changes

link memory setting to node setting

2c2e2d7

gabritto force-pushed the main branch from 03d99c7 to 2c2e2d7 Compare August 24, 2023 01:36

Merge remote-tracking branch 'upstream/main'

5944aae

mjbvz modified the milestones: August 2023, September 2023 Aug 29, 2023

mjbvz enabled auto-merge (squash) September 6, 2023 19:07

mjbvz approved these changes Sep 6, 2023

View reviewed changes

Tyriar approved these changes Sep 6, 2023

View reviewed changes

mjbvz merged commit efc1b30 into microsoft:main Sep 6, 2023
5 checks passed

RyanCavanaugh mentioned this pull request Sep 15, 2023

TS functionality keeps dying on large project microsoft/TypeScript#44951

Closed

kaisalmen mentioned this pull request Sep 25, 2023

Update to vscode v1.82.0 monaco-editor v0.43.0 CodinGame/monaco-vscode-api#189

Merged

github-actions bot locked and limited conversation to collaborators Oct 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom Node option to run TS Server #191019

Add custom Node option to run TS Server #191019

gabritto commented Aug 22, 2023 •

edited

Loading

jakebailey Aug 22, 2023

gabritto Aug 22, 2023

jakebailey Aug 22, 2023

gabritto Aug 22, 2023

jakebailey Aug 22, 2023

gabritto Aug 22, 2023

jakebailey Aug 22, 2023

mjbvz Aug 23, 2023

gabritto Aug 24, 2023

gabritto Aug 24, 2023

jakebailey Aug 24, 2023

gabritto Aug 24, 2023

gabritto commented Aug 28, 2023

Titozzz commented Sep 19, 2023

gabritto commented Sep 19, 2023 •

edited

Loading

gabritto commented Sep 20, 2023

Titozzz commented Sep 20, 2023

gabritto commented Sep 20, 2023

Add custom Node option to run TS Server #191019

Add custom Node option to run TS Server #191019

Conversation

gabritto commented Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabritto commented Aug 28, 2023

Titozzz commented Sep 19, 2023

gabritto commented Sep 19, 2023 • edited Loading

gabritto commented Sep 20, 2023

Titozzz commented Sep 20, 2023

gabritto commented Sep 20, 2023

gabritto commented Aug 22, 2023 •

edited

Loading

gabritto commented Sep 19, 2023 •

edited

Loading