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 configuring tide to avoid starting tsserver immediately #294

Closed
lddubeau opened this issue Jan 9, 2019 · 4 comments
Closed

Allow configuring tide to avoid starting tsserver immediately #294

lddubeau opened this issue Jan 9, 2019 · 4 comments
Assignees

Comments

@lddubeau
Copy link
Collaborator

lddubeau commented Jan 9, 2019

This is a suggestion for a new feature.

The Problem

I use session management to record my Emacs session and restore it when I start Emacs again. I've used this setup for years. The only time I've had a problem before was with flymake but I fixed that by switching to flycheck. However, recently tide has started giving me issues, indirectly. I have a hand in a lot of different TypeScript projects. Restoring a session has become really slow due to all the tsserver instances that are immediately started and compete for CPU. It has been a problem for a long time but recently it's become unbearable. Maybe TS 3 has features that cause tsserver to need more processing power. I don't know. I've definitely been doing things that require me to restart Emacs more often lately. Right now, when I restart Emacs I have pkill -f tsserver at the ready to kill all tsserver instances. (Fortunately, Emacs is the only tool I use that starts tsserver.) Then I start them manually as needed.

The Proposal

I'd like to be able to set a configuration variable such that the variable can be set to tell tide-setup to skip starting tsserver immediately. I'd be perfectly happy with having to issue tide-restart-server manually (like I'm doing now) whenever I need it. I thought about starting tsserver lazily: whenever tide needs it and tsserver is absent, then tide would start it automatically, but I'd be happy even without this functionality.

I volunteer for implementing this, but if there are concerns or flat out objections I'd rather hear about them ahead of time.

@lddubeau lddubeau self-assigned this Jan 9, 2019
@ananthakumaran
Copy link
Owner

ananthakumaran commented Jan 10, 2019

I am not sure what will start to break when we don't start the tsserver. So probably you can come up with some approach and we could discuss the details.

One thing you could try is to move all the variable setup etc into tide-mode function and leave tide-setup with

(defun tide-setup ()
  "Setup `tide-mode' in current buffer."
  (interactive)
  (tide-start-server-if-required)
  (tide-mode 1)
  (tide-configure-buffer)))

this will give you more control over when to start the server. if we just disable the server-start, the tide-configure-buffer is going to break right away as it will try to send command to tsserver. I don't know how other plugins will interact as well, will flycheck try to do error check when the session is restored?

@josteink
Copy link
Collaborator

I would assume there are various kinds of invocations which I would assume only occurs on actual end-user interaction:

  • completion at point
  • fetching eldoc
  • looking up documentation

Etc.

Maybe we can try to lazy-load tsserver (and "configure" the buffer) when these kind of functions get invoked?

And that works out well... Maybe we won't need a preference at all?

@lddubeau
Copy link
Collaborator Author

@josteink

And that works out well... Maybe we won't need a preference at all?

I do prefer reducing the number of configuration variables as much as possible, but sometimes competing considerations weight more in the balance.

Getting lazy-starting right is doable. However, I'm concerned about getting it right in various use-case scenarios that may be hard to predict. I'd be thrilled to put out a one-shot release that nails it in one go, but I'm not confident that will happen. The nightmare scenario is releasing something that works in the test suite, "works for me" (tm), and maybe works for all three of us, but once released we get a flurry of issues because a substantial part of the user base runs into trouble because they happen to do things I did not predict. I discovered that NaN was an issue in #279 purely by chance. The code worked fine, until I switched laptops and started getting CPU percentage numbers that were NaN. I can see that PR having been released and immediately getting issue reports that it crashes for some folks. If that had happened, one saving grace was that tide-list-servers is not a core function of tide so it not working for some users is not a disaster. There's no such saving grace if tide switches to lazy-starting tsserver as its only mode of operation and it does not work right. This is core functionality.

A naive implementation would lazy-start tsserver whenever a tide function that needs tsserver is invoked. But this would prevent users from mitigating annoyances like #268. Right now, if I don't want the extra tsserver I can kill it and it remains killed no matter what I do in the buffer(s) that depend on that dead tsserver. If lazy-starting is mandatory, then when I'd go back to the buffer(s) tide would "helpfully" start tsserver for me and undo what I did.

A less naive implementation would determine some conditions under which it lazy-start should be turned off and require the user to manually invoke tide-restart-server. One such condition could be "the user manually killed tsserver". In such case, when the user invokes a function that needs tsserver the user would get an error message requesting that tsserver be manually started. (Which is what happens to me right now when I manually kill tsserver instances.) There are probably other conditions that should apply. For instance, if it seems tsserver cannot even be started at all, turn off lazy-start instead of trying to start it over and over again. (It could be that the user did not configure paths correctly, or has an faulty Node installation, or some other fundamental error.) There may be yet more conditions I'm not thinking of.

@ananthakumaran You are bringing up issues that I'll have to address. Your observation about how plugins may behave is actually one other reason I'm skittish about the prospect of making lazy-starting tsserver the one method by which tide starts tsserver. I do use flycheck so it is easy for me to see what happens with it. However, there are certainly other tools that should integrate nicely with tide that I don't use.

@lddubeau
Copy link
Collaborator Author

Oops... forgot to close this when #303 was merged.

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

3 participants