tide
tide copied to clipboard
Only start one tsserver, not one per tsconfig
This is how tsserver is intended to be used, according to Typescript team members who work on it. This improves the usability of a long-lived emacs instance after opening many different projects.
I'm testing the change right now and so far performance is better. I don't know whether there are deep-seated reasons to have one tsserver per project root, so I thought that we could discuss that with this PR.
I'm not sure whether initialising tide-server
with defvar
to nil
and updating it with setq
are right since I don't know elisp that well.
Historically tide only supported working against one particular version of tsserver (shipped with tide or overridden, custom path set by the user). In such circumstances, this change would make sense.
Now however, tide defaults to using the same tsserver as provided by the typescript npm package installed in each and every project it works with.
This ensures tide handles things consistently with the projects specific version of tsc, the typescript compiler, automatically.
This was a “big” (although fairly silent) feature with lots of discussion and code-review in the PRs.
Having only one instance of tsserver would directly invalidate all that work invested in that undertaking, so that’s not immediately desired, at least not for all users.
While I can see the argument for preserving resources, it would have to be under very specific constraints, like always using a globally installed tsserver as opposed to the projects’ own.
What about one tsserver per tsserver location instead of one per project? tide-servers
could use tide-locate-tsserver-executable
to get the hash key instead of tide-project-name
.
I have some repos that have multiple tsconfig.json
files. Often, there's one for the application proper, and one for running the test suite. Something like
src/foo.ts
src/tsconfig.json
test/foo.spec.ts
test/tsconfig.json
Way back when, tsserver
did not play well with this kind of setup. If I started editing src/foo.ts
and then moved to edit test/foo.spec.ts
, then I would get behavior out of tsserver
that was consistent with it incorrectly using src/tsconfig.json
for processing test/foo.spec.ts
instead of correctly using test/tsconfig.json
. And if I opened the files in the reverse order then src/foo.ts
was not handled properly.
This, in combination with issue reports like this one (this comment, specifically), gave me the impression that tsserver
is not designed to handle multiple tsconfig.json
files at the same time.
@lddubeau Are you saying the current behaviour of hashing tsservers on tide-project-name
is better or worse than hashing on tide-locate-tsserver-executable
? Or would the bug you linked be the same no matter how many tsservers you start?
@sandersn The current behavior of hashing on tide-project-name
is better than hashing on tide-locate-tsserver-executable
. Hashing on the location of tsserver
in the scenario I gave above would result in a single tsserver
handling source files that are governed by different tsconfig.json
files, and in my experience, that did not work.
Hashing on tide-project-name
means one instance of tsserver
per tsconfig.json
. tide-project-name
looks up the file system for a tsconfig.json
and then takes the basename of the directory that contains the tsconfig.json
and appends a hash to it to work around clashes. In the example I gave above, there would be two project names: src-[some hash]
and test-[some other hash]
, and two instances of tsserver
.
Assuming that tsserver still has the same problem from Microsoft/TypeScript#5828, maybe we should wait until Typescript supports dependencies between tsconfigs, which should be coming in 3.0 or 3.1. That feature exists to make src
and test
relate correctly.
To recap, it seems like there are two reasons to start one tsserver per tsconfig.
- Support local installs of typescript.
- Work around src-vs-test-interaction bugs in tsserver.
If/when (2) is fixed, then I think hashing on tide-locate-tsserver-executable would be more efficient and therefore strictly more desirable, right?
@sandersn Yep, when the 2nd point is fixed it will be better to hash per location to avoid starting so many instances.
@lddubeau Also, if it's been a long time since you observed the bug, it may have been fixed. You can test it by using a constant hash key like "DUMMY" in the gethash
line in tide-current-server
and the puthash
line in tide-start-server
.
I realise it's an imposition to ask you to test whether a bug has been fixed, but I'd like to solve real bugs now (for me, running out of file handles when running VS code at the same time as Emacs) instead of avoiding old bugs that have been fixed.
We could introduce a config tide-reuse-tsserver
and set it to nil by default until all the bugs are sorted out in tsserver. If enabled, it would search for existing tsserver process with same path and reuse it.
Codewise, we assume one server would be used by one project in some places like tide-restart-server
, tide-net-sentinel
. We also store the project-name
in process.
(process-put process 'project-name (tide-project-name))
I guess we could convert it into project-names
and also add another process property tsserver-path
which could be used to find server for reuse.
@sandersn I tried it just now. It is still buggy. Initially, it looks like it is working but I soon get bizarre behavior, like hitting 100% CPU on tsserver
until I kill it. It all goes away when I revert to tide's stock behavior.
@ananthakumaran Yes, I was thinking it should be configurable. It is very likely that users of tide won't be able to move all their projects at once to the new version of TS that has a tsserver
that handles multiple projects without error. These users will want to use the current method of starting tsserver
until they no longer have to deal with the old versions.
Further experimentation with this branch revealed bad behaviour for my Typescript-compiler-development scenario as well (details below). I don't have time for investigation now, so I guess I'll live with the resource usage.
- Point tsserver to
require("~/ts/built/local/tsserver.js")
- Open two projects: ~/ts/src/compiler and ~/test. The locally-built tsserver starts once, for the first project.
- Rebuild the tsserver at ~/ts/built/local/tsserver.
- Switch to a source file in ~/ts/src/compiler and request the type of something. The minibuffer shows part of a stack trace.
- tide-restart-server. Now tsserver is working in ~/ts/src/compiler.
- Switch to a source file in ~/test and request the type of something. The minibuffer again shows part of a stack trace.
- tide-restart-server. No tsserver is working in ~/test.
- Goto 4 or give up.