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

Allow a client to configure typescript.tsdk (similar to VS Code) #64

Closed
keyboardDrummer opened this issue Sep 7, 2018 · 4 comments
Closed

Comments

@keyboardDrummer
Copy link
Contributor

Have you considered using the Session class inside the LSP server process, instead of starting a separate tsserver process?

Pros:

  • Less processes, less communication overhead
  • The LSP server now has access to the internal state of tsserver, so you don't have to store duplicate state in the LSP server, which is done here to store the state of open documents. Note that I believe this is done incorrectly, see this issue.

Cons:

  • Less reliable contract of the dependency, since tsserver's contract is less likely to change than that of Session. This also means that you should always use the Session class that is bundled with the typescript-language-server installation, so you know for sure that it works well. The downside here is that you won't get automatic TypeScript upgrades if the user installs a newer global version of TypeScript. However, I think that behavior was a bit risky to begin with, since the contract of tsserver can also change.
@akosyakov
Copy link
Contributor

I don't think it is a good idea to rely on own tsserver, as you mentioned it won't be forward-compatible.

@keyboardDrummer
Copy link
Contributor Author

keyboardDrummer commented Sep 11, 2018

Well, do you think tsserver tries to be backwards compatible? I don't see what guarantee you have that it will be. It seems dangerous to me to just pick a tsserver from the environment as you do here, since it might be a newer version than what you have tested against. It seems safer to me to bundle a tsserver installation, and if you're doing that, then you might as well use the code in-process, instead of starting up a separate tsserver.

A related issue is that if you support LSP's workspaceFolders feature, then you won't be able to use the tsserver from the project's node_modules, as you do here, since there will be multiple projects. See this issue.

Anyways if you strongly oppose this suggestion, then feel free to close this issue.

@akosyakov
Copy link
Contributor

Well, do you think tsserver tries to be backwards compatible?

We are trying to align behavior with the VS Code extension. If they can assume that tsserver is backward compatible, I think we can too.

A related issue is that if you support LSP's workspaceFolders feature, then you won't be able to use the tsserver from the project's node_modules, as you do here, since there will be multiple projects. See this issue.

In VS Code a user is responsible to decide which tsserver should be used via workspace configurations. We can rely on LSP configurations capabilities as well. In the case, if a configuration is not provided we can either prompt a user via the custom request or a fall-back to the first tsserver.

@akosyakov akosyakov changed the title Consider using internals of tsserver instead of starting it as a separate process Allow a client to configure typescript.tsdk (similar to VS Code) Sep 11, 2018
@akosyakov
Copy link
Contributor

It's already possible with --tsserver-path option. The tool can decide which tsserver to use and whether to let a user switch between them.

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

No branches or pull requests

2 participants