Repository navigation
Vue LS: honour ls_path/ls_base_cmd and require npm only for installing - #2177
Open
karajanchang wants to merge 1 commit into
Open
karajanchang wants to merge 1 commit into
karajanchang wants to merge 1 commit into
Conversation
opcode81
requested changes
Oct 19, 2026
Comment on lines
+563
to
+571
| base_cmd = vue_config.get("ls_base_cmd") | ||
| if base_cmd is not None and not isinstance(base_cmd, list): | ||
| log.warning("The 'ls_base_cmd' setting should be a list of strings. Ignoring the provided value: %s", base_cmd) | ||
| base_cmd = None | ||
| if base_cmd is None: | ||
| ls_path = vue_config.get("ls_path") | ||
| if ls_path is None: | ||
| return None | ||
| base_cmd = [ls_path] |
Member
There was a problem hiding this comment.
ls_path and ls_base_cmd support are not supposed to be implemented directly in a language server wrapper. Instead, they are centrally provided via LanguageServerDependencyProviderBaseCommand and its subclasses.
The Vue language server is an old implementation which predates the introduction of the LanguageServerDependencyProvider abstraction. To add these features, the implementation should be refactored to use a dependency provider instead of adding custom management here.
See also: memory adding_new_language_support_guide
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2176 (fixes 1 and 2 from that issue).
VueLanguageServerresolves its runtime dependencies in_setup_runtime_dependenciesbeforeSolidLanguageServer.__init__runs, so the genericls_path/ls_base_cmdoverrides fromLanguageServerDependencyProviderBaseCommandnever apply to Vue. As a result, the only way to reuse anexisting installation is to recreate the internal
vue-lsp/layout and write a matching.installed_versionmarker. On top of that, npm is required even when nothing has to be installed.Changes
ls_path/ls_base_cmdfor Vue. If either is set inls_specific_settings["vue"], Serenauses it as the base command (
--stdiois appended, as in the managed case), installs nothing, andrequires neither node nor npm. Vue also has to know where its companion pieces are, so three
settings go with it:
ts_ls_path: thetypescript-language-serverexecutabletsdk_path: the TypeScriptlibdirectory (also used for the tsserver pin in untrusted projects)vue_plugin_path: the@vue/typescript-plugindirectoryIf any of them is missing, a
SolidLSPExceptionlists them by name. Silently falling back to themanaged paths would bring the npm install back, and with it the problem being fixed.
Precedence matches the generic provider:
ls_base_cmd(must be a list, otherwise warned about andignored) wins over
ls_path.npm only for installing. For the managed installation, node is still checked up front (the
shims need it to run), but npm is only checked once
needs_installis true. Both checks now raiseSolidLSPExceptioninstead of usingassert, which is skipped underpython -O.To carry the extra path,
_setup_runtime_dependenciesnow returns a small frozenVueRuntimePathsdataclass instead of a 3-tuple.
@vue/typescript-pluginwas previously derived from the manageddirectory inside
_start_typescript_server; it is now part of the resolved paths, which also letsself._vue_ls_dirgo. The managed code path behaves the same as before: same directory layout, same.installed_versionmarker, same defaults.Not changed
.installed_versionformat and the default versions.VueTypeScriptServer(it already took explicit paths through its constructor).Testing
New
test/solidlsp/vue/test_vue_runtime_dependencies.py. It needs no node, npm or network:shutil.whichand
RuntimeDependencyCollection.installare monkeypatched, and installs are recorded so the tests cancheck that none happened.
ls_pathwith all companion paths → the expectedVueRuntimePaths, no install, no node/npmls_base_cmdtakes precedence overls_pathls_pathwith companion paths missing →SolidLSPExceptionthat names the missing keysSolidLSPException("npm is not installed"), no installSolidLSPExceptionRun against unmodified
main, the new test module fails at collection (VueRuntimePathsdoes notexist yet).
poe format,poe lintandpoe type-checkpass.tyreports one existing warning, an unusedty: ignoreatsrc/serena/dashboard.py:1078. That file is not touched by this PR.End-to-end on macOS (arm64), against a real Vue LS:
ls_path,ts_ls_path,tsdk_pathandvue_plugin_pathat an existing install(
@vue/language-server3.1.5, TypeScript 5.9.3,typescript-language-server5.1.3).PATHheld onlynode. npm was not onPATHat all.test/resources/repos/vue/test_repo/src/App.vue.CHANGELOG.mdhas an entry under Unreleased → Language Servers.Generated with MUR