Skip to content

Add build target for packaging tsserver as a library#3513

Closed
weswigham wants to merge 4 commits into
microsoft:masterfrom
weswigham:language-service-lib
Closed

Add build target for packaging tsserver as a library#3513
weswigham wants to merge 4 commits into
microsoft:masterfrom
weswigham:language-service-lib

Conversation

@weswigham

Copy link
Copy Markdown
Member

Specifically, adds jake lssl (language service server library). The goal was to export the Session type which, in turn, has necessitated exporting most of the types in editorServices.ts by extension.

Additionally, I've made some changes to enable this library to function outside node - (node.d.ts is no longer compiled into the lssl target by any of the files included in it). Specifically, the two features of Node which Session used - Buffer.byteLength and process.hrtime, I've offloaded to a new Environment interface, and created a node implementation of it for tsserver - server\nodeimpl.ts. I was considering that they may be better provided by the System interface, and would like feedback on this - changing the System interface seemed like a farther reaching change than was desired.

On top of this, to make Session (or trivial subclasses) actually instantiate at runtime in non-node environments, I removed Session's meager dependency on ts.sys - which only gets set in node or in windows script host. For the most part, this simply meant calling the host which Session was constructed with, but in one instance this meant that the host had to be passed along to a static function - ScriptVersionCache.fromString. Additionally, the static CompilerService.defaultFormatCodeOptions had to be made aware that ts.sys may not be set at static initialization time if not running in either node or windows script host.

The remaining changes, b8f6ada and 9a3fead, are less critical to exporting the Session type as a library, but do provide quality improvements in the exported interface.

  • b8f6ada allows consumers of the Session class to subclass and override onMessage and still call the onMessageParsed implementation - or just call onMessageParsed directly. This is useful in situations where the service is capable of receiving message objects as objects directly, rather than as json strings, thereby allowing consumers to remove (de)serialization overhead.
  • 9a3fead adds support into the new onMessageParsed function to handle arbitrary messages, rather than throwing immediately if the massive switch statement falls all the way through. (IMO, it may be nicer to register all message handlers this way and remove the switch entirely.) This is incredibly useful in situations when the session communication channel is intended to the the primary communication channel between a client and a server, and allows a server to provide extended functionality (or, in the case of running in a browser worker thread, allows a client to send the server content with which to build a virtual filesystem and external edits to the files therein). This is the least required of the changes herein, so if too much issue is taken with it, it is not necessarily required if the prior commit is still accepted.

@msftclas

Copy link
Copy Markdown

Hi @weswigham, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftclas

Copy link
Copy Markdown

@weswigham, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

Comment thread src/server/nodeimpl.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

these are only used in the session object, i do not see the point of the additional abstraction, just pass them as constructor paramters.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's pretty much what this is doing, it was just bundling the two parameters into one argument. I'll split it out into multiple arguments.

@mhegazy

mhegazy commented Jun 17, 2015

Copy link
Copy Markdown
Contributor

@weswigham can you split this into two, pre b8f6ada and 9a3fead, i would want to discuss them first.
I do not think we need the extra layer of abstraction for nodeEnviroment.

weswigham added a commit to weswigham/TypeScript that referenced this pull request Jun 17, 2015
@weswigham

Copy link
Copy Markdown
Member Author

Done - see #3542 for the library packaging part. And #3543 for the interface improvements.

@weswigham weswigham closed this Jun 18, 2015
@weswigham weswigham deleted the language-service-lib branch August 17, 2017 23:00
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants