Add Svelte language server support with TypeScript integration #504
Conversation
- Add SvelteLanguageServer implementation using svelte-proxy-lsp - Support for .svelte, .ts, .js, .tsx, .jsx files and SvelteKit variants - Multi-project detection based on tsconfig.json files - Automatic installation via npx - Comprehensive test suite and example project
- Adjust test expectations to match current svelte-proxy-lsp capabilities - Skip completions test until fully supported - Tests now passing with 6 passed, 1 skipped
- Skip all Svelte tests on Windows platform due to npx/npm setup requirements - Add better error handling when npx is not available - Tests still run on macOS and Linux CI environments
There was a problem hiding this comment.
Thanks, it sounds great, and would be awesome if the svelte support could be added in such an easy way.
But we need a few more things before this can be merged.
Serena starts a local server on the user's code, so it is very important to be transparent about what kind of stuff is started. I can't find info on the package on npmjs.com. I suppose the repo where it's developed is this one, right? If we include such external dependencies, we need to completely pin them, write detailed comments on what they do and provide links. Otherwise, we open ourselves up for security problems. I generally feel a bit uneasy about starting code that is neither an established LS nor part of Serena's repo on the users' machines. So before merging this, we'd need do at least an audit of your project, or we can think of including the source here in some form. There is currently no precedence for relying on external code in such a way in Serena. Maybe the best solution would be porting the logic of your proxy to python. @opcode81 WDYT?
Btw, another approach would be to finalize #361 and document that users can use your project for svelte via this approach - then "at their own risk". This is of course less nice from the user perspective, but bypasses the complications outlined above.
Concerning the functionality, the hardest part to get right was the cross-language reference finding. There are no tests for that in your PR. We also shouldn't have that pretend to work but in reality don't test anything. Moreover, tests running on windows is important - we can't just skip them. If additional setup is necessary, it should be done in the runner.
|
Totally understand the concer about a random 3rd party depedency, I can have a look at porting the logic of LSP to python. I actually had to push my changes to check my windows machine to see if it was working and didn't mean to push back into the PR. I do prefer the custom LSP approach though, I'll take a look at that instead I think as it clearly is far more inline with what I want to do, and makes future LSP changes (either on ts or svelte) updaes easier to manage locally. |
|
For implementing the custom LSP support a small architectural extension is missing that will help with it and that I wanted to push in the next days. So please wait for that before exploring that option :). A python port would definitely be the best option for "vanilla" users of Serena who just want it working for svelte |
|
See my comment on #339 |
|
Hey, |
|
@datstarkey is this abandoned? |
5 participants
Strongly based on: #339
I had the idea of merging the svelte lsp and ts lsp as a proxy a long time ago for use for llm diagnostic checking , but never finsihed the project.
Based on the abandonment of the previous pull request, I finished up the project and published it to npm. The full repo is available there has has more extensive tests:
https://github.com/datstarkey/svelte-proxy-lsp
I've done some basic tests and checs with my projects and it seems to be working, additonal support or guidane would be appreciated on anything wrong! still pretty new with lsp's, but it looks like its working. I have used it as a npx request, as I ran into issues installing it.