TSServer

[basarat]

Keep an eye on microsoft/TypeScript#2041

Might significantly reduce the amount of architecture we need to maintain.

[basarat]

Code Review:

• Also all the TS Server code is in here : https://github.com/Microsoft/TypeScript/tree/master/src/server
• TSServer has its own implementation of watchFile : https://github.com/Microsoft/TypeScript/blob/master/src/server/server.ts#L201-L208 for good reasons mentioned in the comment above the code
• another sample editor https://github.com/Microsoft/ngconf2015demo/blob/master/sublimetext/typescript/TypeScript.py
  • class EditorClient sets up an instance of TSServer
• useful read : https://github.com/Microsoft/TypeScript/blob/master/src/server/protocol.d.ts as far as I can tell it doesn't have full bidirectional communication
• I only see request : client -> server response : server -> client (for a request), event : server -> client (first and forget for the server really)
• It also uses readline to communicate with client : https://nodejs.org/api/readline.html : https://github.com/Microsoft/TypeScript/blob/master/src/server/server.ts#L10-L14
• A nice implementation of script snapshot here : https://github.com/Microsoft/TypeScript/blob/master/src/server/editorServices.ts
• Limits power significantly as you do not have the complete language service access e.g. getProgram is blocked out : https://github.com/Microsoft/TypeScript/blob/master/src/server/client.ts#L479-L485 this is vital for the next iteration of advanced features.

[basarat]

Final opinions ( @park9140 / @csnover ). Recommend we don't use TSServer

• Doesn't support two way so don't see a graceful error recovery. Things will crash. I'd rather that the server pulls information rather than we pushing.
• Limits user power. You don't get the complete language service. Some features will need program which isn't serialized.

Good stuff we should pull in:

• Watches files. We should watch the files as well. And use polling not system hooks.
• Has a really nice and efficient script snapshot and language service host implementation we should bring that in. This was already a part of the plan.

[csnover]

Make sure that the TypeScript team receives this feedback, if the server doesn’t work for atom-typescript it probably won’t work for other people that want to use it too. I don’t understand what you are saying about “full” bidirectional communication or “doesn’t support two way”, so it would be useful for me if you could explain exactly what that means and why you think the tsserver communication model doesn’t work.

[basarat]

I don’t understand what you are saying about “full” bidirectional communication or “doesn’t support two way”

Command -> response is only client -> server -> client. There is no server -> client -> server mechanism. There is however a server -> client mechanism (called event) which is one way first and forget.

This isn't a big blocker though. But we use it here : https://github.com/TypeStrong/atom-typescript/blob/master/lib/main/lang/projectService.ts#L88-L93

[basarat]

The real blocker is just that the server abstracts away the language service and only gives us Object bags and not Class instances, it really can't do that easily over stdout. The class instances only exist in the worker (true in our case as well) but since we wrote the worker we can extend it with new stuff whereas if we use the TSServer worker (called the server) we can't extend the worker. TSServer is really just a reference implementation of a worker.

I'll raise an issue with the TypeScript team right now.

[csnover]

My current thinking is that the server shouldn’t be querying the client for information like that; it should be a unidirectional model. The server lets the client know when an event happens and then if the client needs to change something in the server it can do it through the normal request/response channel. This makes operations non-atomic, but, it probably isn’t a big deal. Otherwise, the server starts mandating that a client implement certain operations, which doesn’t seem right. Does that make sense, or am I not thinking of something correctly?

[csnover]

Oh, and with regards to the missing types, is this really an issue? I would expect the client just implements a reduced subset of types anyway that expose API that’s good for the client itself.

[steveluc]

We are big fans of atom-typescript. It's the kind of great tooling we had in mind when we built the language service. The protocol-based system is there for editors, like Emacs and Sublime, that do not have native JavaScript. As you point out, the snapshot system and watch code is to reduce requirements on plug-ins. Makes sense to use the broader language service and compiler APIs when editor has native JavaScript, especially with a project as ambitious as atom-typescript.

[basarat]

Otherwise, the server starts mandating that a client implement certain operations, which doesn’t seem right. Does that make sense

I agree completely. But having that ability of two way communication made it easier to write (potentially bad in that its strongly coupled) simple code to just fetch the data it needs. Like I said it could be refactored later and not really a blocker.

@steveluc thanks for the clarification ❤️ will keep going as planned.

[park9140]

Seems like we should really rely on as much from core as possible. In this case however we definitely need more access.

Looking through the way things are done we can do way better efficiency wise by implementing the language service script snapshots for open files on top of changes sent by the editor rather than tracking by file watches where we need to parse for diffs in order to get change sets.

As far as I can see we actually need to do this in order to get live edit efficiency for things like format on keystroke to work.

[basarat]

We definitely need to do diffs for open files as you've mentioned but we do need tracking by file watches where we need to parse for diffs in order to get change sets because of branch hopping (something that @csnover loves to do I hear) where the editor doesn't notify us of changes to filesets.

[basarat]

We have a new milestone : https://github.com/TypeStrong/atom-typescript/milestones/5%20TSServer

[csnover]

something that @csnover loves to do I hear

:) it has been known to happen.

[steveluc]

FWIW with Sublime and Emacs, we do our best to send individual diffs (location, deleted length, inserted string) to the server. Emacs gives us diffs for every change. ST3 gives us on_text_command, on_modified, and on_post_text_command. However, for example, none of these are called on undo/redo. Sometimes only on_modified is called, usually for the insert command. ST3 also has view.change_count(), which is incremented after undo. ST2 gives less information and so more buffer chunks need to be sent. For large files, sending the individual changes leads to significantly smoother performance.