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

Allowing starting tsserver as a module using cp.fork #88

Conversation

keyboardDrummer
Copy link
Contributor

Previously, tsserver could only be started as an executable, which would often run the .bin/tsserver file that uses a Node shebang to start tsserver.js using the first Node on the $PATH.

With this PR, tsserver can also be started as a module, by setting tsserverPath so it points to tsserver.js. Also, when tsserver is bundled with the TS LSP server, it will be started using cp.fork, so it uses the same version of Node as the TS LSP server.

This PR is related to #78

@gitpod-io-legacy-app
Copy link

Open in Gitpod - starts a development workspace for this pull request in code review mode and opens it in a browser IDE.

@@ -102,7 +102,10 @@ export class TspClient {
this.cancellationPipeName = tempy.file({ name: 'tscancellation' } as any);
args.push('--cancellationPipeName', this.cancellationPipeName + '*');
this.logger.info(`Starting tsserver : '${tsserverPath} ${args.join(' ')}'`);
this.tsserverProc = cp.spawn(tsserverPath, args);
const tsserverPathIsModule = path.extname(tsserverPath) === ".js";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should support ".mjs" as well

@mattlyons0
Copy link
Contributor

So this is a more conservative version of #78, I like it. The trade off is that you actually have to point to typescript/lib/tsserver.js instead of .bin/tsserver since the typescript package uses a lot of shebang syntax, but this way when you are calling a .js file you explicitly know you are getting the same node binary to run. Maybe add a note around the --tsserver-path flag in README.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants