kabouzeid / nvim-lspinstall

Provides the missing :LspInstall for nvim-lspconfig
MIT License
526 stars 67 forks source link

Incomplete angular installer #72

Open jemag opened 3 years ago

jemag commented 3 years ago

From my, very rudimentary, understanding of the angular language server I believe the current config may be incomplete (feel free to correct me if that is not the case).

Currently the script, will install the language server and update the default cmd. However, if we want to make use of the angular language server in a portable fashion, I believe it requires a typescript installation. So if I were to try and use it from its installed folder, it will trigger an error like so :

node node_modules/@angular/language-server/bin/ngserver --stdio --ngProbeLocations ./
/home/jemag/.local/share/nvim/lspinstall/angular/node_modules/@angular/language-server/index.js:157
      throw new Error(`Failed to resolve '${packageName}' with minimum version '${minVersion}' from ` + JSON.stringify(probeLocations, null, 2));
      ^

Error: Failed to resolve 'typescript/lib/tsserverlibrary' with minimum version '4.2' from [
  "--help"

Also the command does not specify the --tsProbeLocations and --ngProbeLocations. So if I am in an angular project called "Neptune" , LspInfo will return the following command used: image

Which will not work if the project does not have @angular/language-service installed, or if the project uses a Typescript version < 4.2

Basically, I think it would make sense to also install typescript and properly set the --tsProbeLocations and --ngProbeLocations to the nvim-lspinstall's installed typescript and angular/language-service. Perhaps leaving the freedom to the user to change those, if needed.

For example, this was the previous basic setup that I was using to make angularls work:

local configPath = vim.fn.stdpath("config")
local languageServerPath = configPath.."/languageserver"
local angular_cmd = {"node", languageServerPath.."/node_modules/@angular/language-server/index.js", "--stdio", "--tsProbeLocations", languageServerPath, "--ngProbeLocations", languageServerPath}

require'lspconfig'.angularls.setup{
  on_attach = on_attach_common,
  cmd = angular_cmd,
  on_new_config = function(new_config,new_root_dir)
  new_config.cmd = angular_cmd
  end,
}

where languageserver is a folder containing the required dependencies (angular/language-server and typescript).

P.S.: For some reason, even with projects having typescript >4.2 and @angular/language-service installed, the current default configuration did not seem to work. Curious if you've made it work for some test angular projects?

kabouzeid commented 3 years ago

Yes, I made it work by installing angular language service and typescript for each project that is using it. I'll try again, see if it still works.

iirc this is also what the VSCode extension does, and I think it's the only correct approach. I can't assume what specific version of typescript (or angular) a project is using.

jemag commented 3 years ago

Seems the emacs guys are using a similar approach (https://github.com/emacs-lsp/lsp-mode/wiki/Install-Angular-Language-server), although with globally installed @angular/language-service, typescript and @angular/language-server (I personally prefer to avoid globally installed npm packages whenever possible)

tomtomdurrant commented 3 years ago

So is the advice to install typescript globally on the system? Having to install the angularls inside each project seem to be slightly contradictory to the point of the LspInstall extension

mactep commented 2 years ago

iirc this is also what the VSCode extension does, and I think it's the only correct approach. I can't assume what specific version of typescript (or angular) a project is using.

VSCode extension ships the language service and typescript within itself as stated in the README.

This code seems to work with the language server shipped with lspinstall.

local config = require"lspinstall/util".extract_config("angularls")

local cmd = { "./node_modules/.bin/ngserver", "--stdio", "--tsProbeLocations",  "./node_modules", "--ngProbeLocations", "./node_modules" }

config.default_config.cmd = cmd
config.default_config.on_new_config = function(new_config,new_root_dir)
    new_config.cmd = cmd
end

return vim.tbl_extend('error', config, {
  install_script = [[
  ! test -f package.json && npm init -y --scope=lspinstall || true
  npm install @angular/language-server @angular/language-service typescript
  ]]
})

Just to mention that this will not work on legacy projects as per this issue. I didn't get to a proper solution, if someone is interested in helping.

lucasvianav commented 2 years ago

hey @mactep. this edited config that is being returned is the one that would be passed to require('lspconfig').angular.setup?

I can't get it to work, everything I try gives me Error: cmd not defined for "angular". You must manually set cmd in the setup{} call according to CONFIG.md. when starting LSP.

mactep commented 2 years ago

Salve.

Actually I changed this file.

But if you want to get it working, just do something like this. Just change languageServerPath to the directory you installed @angular/language-service and typescript.

lucasvianav commented 2 years ago

Great, I'll try it later today. Obrigado querido :)