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

Extract YAML language service to another repo? #104

Closed
pengx17 opened this issue Nov 19, 2018 · 7 comments
Closed

Extract YAML language service to another repo? #104

pengx17 opened this issue Nov 19, 2018 · 7 comments

Comments

@pengx17
Copy link
Contributor

pengx17 commented Nov 19, 2018

As far as I know, yaml-language-server was invented to make a YAML plugin for code editors (especially vscode), and it is running as a standalone Node.js application in the extension.

To make it running inside of monaco-editor, the yaml language service has to be imported as a module dependency statically with UMD or ESM. What we are doing currently in monaco-yaml here is to copy /src/languageservice/* into our repo as a native module which then could be statically imported, and finally to be built with tsc/vscode bundle helpers. This is clearly not a clean solution.
A better solution in my opinion is to extract the yaml language service into a new repo and publish it into vscode-yaml-language-service. Then both vscode-yaml-languageserver(this repo) and monaco-yaml could depend on it.

What do you think?

@JPinkney
Copy link
Contributor

@gorkem @fbricon WDYT? If i'm understanding this correctly I think the pros are that it helps monaco-yaml but on the flip side it seems like we add another level of indirection for users to report issues and this repo would literally just have the server.ts file and include that new dependency. It would still enable it to be its own standalone node js application but this repo would essentially become a shell.

@fbricon
Copy link
Contributor

fbricon commented Nov 19, 2018

Can't we just publish the server to npm? That should be enough for adopters to depend on it, I don't think it'd require splitting the git repos.
But I reckon I know next to nothing when it comes to modern javascript toolchain.

@fbricon
Copy link
Contributor

fbricon commented Nov 19, 2018

mmm it's already available https://www.npmjs.com/package/yaml-language-server

@pengx17
Copy link
Contributor Author

pengx17 commented Nov 20, 2018

@fbricon it is, but the released bundle is not in a shape which can be imported into a js application statically as a ts lib.
I guess a solution is to have a separate build script to release the service as a typescript library and publish it to npm. Having a separate repo is indeed not necessary any way this way.

@fbricon
Copy link
Contributor

fbricon commented Nov 20, 2018

ok so the npm module must also contain the ts type definitions. I don't know how everything is published to npm, but I guess that's irrelevant, as long as the build script includes those definitions that should be ok. @pengx17 feel free to provide a PR

@pengx17
Copy link
Contributor Author

pengx17 commented Nov 20, 2018

Will make one PR for this later.

@evidolob
Copy link
Collaborator

With #305 this should be solved.

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

5 participants