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

ts-ls: Wrong lsp-clients-typescript-server-path #4254

Closed
2 of 3 tasks
eginhard opened this issue Dec 9, 2023 · 20 comments
Closed
2 of 3 tasks

ts-ls: Wrong lsp-clients-typescript-server-path #4254

eginhard opened this issue Dec 9, 2023 · 20 comments
Labels

Comments

@eginhard
Copy link

eginhard commented Dec 9, 2023

Thank you for the bug report

  • I am using the latest version of lsp-mode related packages.
  • I checked FAQ and Troubleshooting sections
  • You may also try reproduce the issue using clean environment using the following command: M-x lsp-start-plain

Bug description

#4202 made some changes to handle a deprecated argument in the language server. This included changing lsp-clients-typescript-server-path() to return:

(f-join (f-parent (lsp-package-path 'typescript)) "node_modules" "typescript" "lib"))))

For me this evaluates to ~/.emacs.d/var/lsp/server/npm/typescript/bin/node_modules/typescript/lib, while tsserver.js is actually in ~/.emacs.d/var/lsp/server/npm/typescript/lib/node_modules/typescript/lib. So I guess the line above should be:

(f-join (f-parent (f-parent (lsp-package-path 'typescript))) "lib" "node_modules" "typescript" "lib"))))

Steps to reproduce

  1. Open a .js file with ts-ls, the following error appears: LSP :: Error from the Language Server: Request initialize failed with message: Could not find a valid TypeScript installation. Please ensure that the "typescript" dependency is installed in the workspace or that a valid tsserver.path is specified. Exiting. (Internal Error)

Expected behavior

ts-ls starts normally

Which Language Server did you use?

lsp-javascript/ts-ls

OS

Linux

Error callstack

No response

Anything else?

No response

@eginhard eginhard added the bug label Dec 9, 2023
@hamaker
Copy link

hamaker commented Dec 19, 2023

I'm having this issue as well. Reverting the change from #4202 to line 803 resolved it.

@yyoncho
Copy link
Member

yyoncho commented Dec 19, 2023

@kiennq - seems like it is a regression?

@ncaq
Copy link
Contributor

ncaq commented Jan 23, 2024

I am having a rather hard time getting lsp to work in a TypeScript project in a subdirectory due to this problem.
Also, the original PR does not work in environments that do not use node_modules such as yarn PnP, and I expect that some people will have trouble with that.

@idhowardgj94
Copy link

I'm having this issue as well. Reverting the change from #4202 to line 803 resolved it.

same issue here, and resolved by follow @hamaker's suggestion.

I guess that new code didn't include all kind of npm install path.

For my case, I'm using nvm, and my global install path is in ~/.nvm/

And I can use M-x lsp-install-server command, and that tsc & tsserver bin install inside ~/.emacs.d/.cache/lsp/npm/typescript/bin

didn't really deep into what's going on here, though, hope someone can solve this problem :)

@idhowardgj94
Copy link

try to dig deeper into the problem, I think that maybe not related to various of npm install path.

Maybe that logic just wrong in linux environment?

I thought that PR with the problem try to fix issue on window's version

ncaq added a commit to ncaq/lsp-mode that referenced this issue Feb 14, 2024
Stop reading `node_modules` directly by the file system.
The problem with this direct loading is that projects without `node_modules` in the top-level directory will not work at all.
For example, when I develop with aws cdk,
I cut off the `cdk` or `infra` directory and create a new `package.json` and put `node_modules` there too,
but in that case, the code before this modification will not work at all.
I solved this problem by simply inserting an abstraction layer.

Relation.
* [lsp-javascript: supply correct path to tsserver for ts-ls by kiennq · Pull Request emacs-lsp#4202 · emacs-lsp/lsp-mode](emacs-lsp#4202)
* [ts-ls: Wrong lsp-clients-typescript-server-path · Issue emacs-lsp#4254 · emacs-lsp/lsp-mode](emacs-lsp#4254)
@stradicat
Copy link

stradicat commented Mar 5, 2024

I'm getting the same problem in (ubuntu) Linux; also:
Typescript specified through user setting ignored due to invalid path ..completely_wrong_path..

Deleting ~/.emacs.d/.cache/lsp/npm/typescript* and installing typescript and typescript-language-server globally mitigated the problem; the language server isn't being picked up under ~/.emacs.d/.cache/lsp, although eslint and other node-based servers are correctly detected.

This, however, does not happen in macOS; all language servers are correctly detected under ~/.emacs.d/.cache/lsp

@hyperair
Copy link

hyperair commented Mar 6, 2024

On my machine, my emacs.d cache folder looks like this:

~/.emacs.d/.cache/lsp/npm/typescript
├── bin
│   ├── tsc -> ../lib/node_modules/typescript/bin/tsc
│   └── tsserver -> ../lib/node_modules/typescript/bin/tsserver
└── lib
    └── node_modules
        └── typescript

5 directories, 2 files

Similar to the issue description, the elisp statement linked there evaluates to ~/.emacs.d/.cache/lsp/npm/typescript/bin/node_modules/typescript/lib which doesn't exist.

The fix in #4284 works for me, and as a quick workaround without having to locally edit my elpa files, adding a node_modules symlink in the path that lsp-mode is erroneously looking in also works:

$ cd ~/.emacs.d/.cache/lsp/npm/typescript/bin && ln -s ../lib/node_modules

@ghferrari
Copy link

I'm seeing this error on Debian Linux with Emacs 29.2. As a temporary workaround, I've created a symlink via

ln -s <path_prefix>/lsp/npm/typescript/lib/node_modules <path_prefix>/lsp/npm/typescript/bin

NB Amend <path_prefix> to suit the paths in your local installation and see the lsp-log error messages for the path where LSP is expecting to find the node_modules directory.

@ncaq
Copy link
Contributor

ncaq commented Mar 14, 2024

My problem was solved by the PR I sent out last month, does this solve yours?
fix: remove direct references to the node_modules directory by ncaq · Pull Request #4333 · emacs-lsp/lsp-mode

@jcs090218
Copy link
Member

Can someone give this a test? 🤔 Thanks!

@ghferrari
Copy link

Thanks @ncaq - your PR seems to fix the problem for me.

@jcs090218
Copy link
Member

This doesn't look right. If you are on windows, the path looks like: 🤔

~/.emacs.d/.cache/lsp/npm/typescript/node_modules/typescript/lib

@jcs090218
Copy link
Member

Can someone test #4389? It should work on both Unix-like and Windows systems. 🤔

@kiennq
Copy link
Member

kiennq commented Mar 24, 2024

Can someone test #4389? It should work on both Unix-like and Windows systems. 🤔

It looks good to me.

@jcs090218
Copy link
Member

Completed in #4389.

@ncaq
Copy link
Contributor

ncaq commented Mar 27, 2024

I'm referencing the node_modules directory from the lsp root, so the problem isn't solved in environments with node_modules in subdirectories, etc. that my PR solved. should this be a separate issue?

@ncaq
Copy link
Contributor

ncaq commented Mar 27, 2024

Perhaps you are under the mistaken impression that my PR is simply to change the PATH specification of the original issue contributor, while my PR is to stop referring to the PATH directly...
Well, I'll make sure it works on Windows.

@ncaq
Copy link
Contributor

ncaq commented Mar 27, 2024

I have checked it works on Windows and found that it certainly does not work on Windows, even with the content of my PR, so I would like to correct it.

@ncaq
Copy link
Contributor

ncaq commented Mar 27, 2024

@jcs090218
The problem was not completely resolved in my environment, but your point about it not working on Windows was correct, so I have corrected my submitted PR with reference to your correction as well.
I would be happy to review it for you.
fix: remove direct references to the node_modules directory by ncaq · Pull Request #4333 · emacs-lsp/lsp-mode

@jcs090218
Copy link
Member

I'm referencing the node_modules directory from the lsp root, so the problem isn't solved in environments with node_modules in subdirectories, etc. that my PR solved. should this be a separate issue?

It's better to be in separate issues. 👍 The title is a bit misleading. 🤔

jcs090218 pushed a commit that referenced this issue May 26, 2024
* fix: remove direct references to the node_modules directory

Stop reading `node_modules` directly by the file system.
The problem with this direct loading is that projects without `node_modules` in the top-level directory will not work at all.
For example, when I develop with aws cdk,
I cut off the `cdk` or `infra` directory and create a new `package.json` and put `node_modules` there too,
but in that case, the code before this modification will not work at all.
I solved this problem by simply inserting an abstraction layer.

Relation.
* [lsp-javascript: supply correct path to tsserver for ts-ls by kiennq · Pull Request #4202 · emacs-lsp/lsp-mode](#4202)
* [ts-ls: Wrong lsp-clients-typescript-server-path · Issue #4254 · emacs-lsp/lsp-mode](#4254)

* fix(lsp-javascript): remove shell redirect when call node

In the case of Windows, etc., `shell-command-to-string` is to use a non-bash shell.
To begin with, `shell-command-to-string` seems to ignore standard error output.

* refactor(lsp-javascript): rename function and add docs

`lsp-clients-typescript-server-path-by-node-require` is too long.

* fix(lsp-javascript): use Node.js require to explore

Pros
===

Since Node.js require indicates the path where the file actually exists,
it automatically adapts to various environments unless there are significant system changes or changes in the usage environment.

Cons
===

There is a small overhead at first startup due to the command execution.

* refactor: use `if-let*` and `when-let*`

Simplified based on code review.
cheerio-pixel pushed a commit to cheerio-pixel/lsp-mode that referenced this issue May 29, 2024
…p#4333)

* fix: remove direct references to the node_modules directory

Stop reading `node_modules` directly by the file system.
The problem with this direct loading is that projects without `node_modules` in the top-level directory will not work at all.
For example, when I develop with aws cdk,
I cut off the `cdk` or `infra` directory and create a new `package.json` and put `node_modules` there too,
but in that case, the code before this modification will not work at all.
I solved this problem by simply inserting an abstraction layer.

Relation.
* [lsp-javascript: supply correct path to tsserver for ts-ls by kiennq · Pull Request emacs-lsp#4202 · emacs-lsp/lsp-mode](emacs-lsp#4202)
* [ts-ls: Wrong lsp-clients-typescript-server-path · Issue emacs-lsp#4254 · emacs-lsp/lsp-mode](emacs-lsp#4254)

* fix(lsp-javascript): remove shell redirect when call node

In the case of Windows, etc., `shell-command-to-string` is to use a non-bash shell.
To begin with, `shell-command-to-string` seems to ignore standard error output.

* refactor(lsp-javascript): rename function and add docs

`lsp-clients-typescript-server-path-by-node-require` is too long.

* fix(lsp-javascript): use Node.js require to explore

Pros
===

Since Node.js require indicates the path where the file actually exists,
it automatically adapts to various environments unless there are significant system changes or changes in the usage environment.

Cons
===

There is a small overhead at first startup due to the command execution.

* refactor: use `if-let*` and `when-let*`

Simplified based on code review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants