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

lsp-javascript: supply correct path to tsserver for ts-ls #4202

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

kiennq
Copy link
Member

@kiennq kiennq commented Oct 25, 2023

According to the docs, the --tsserver-path is deprecated and we should use the initialization option of tsserver.path instead.
Also, the path should point to tsserver.js or the typescript lib directory.

Currently the newest ts-ls is broken on Windows. This PR fixes that.

@github-actions github-actions bot added the client One or more of lsp-mode language clients label Oct 25, 2023
@yyoncho yyoncho merged commit 808c4d0 into emacs-lsp:master Oct 26, 2023
12 of 14 checks passed
eginhard added a commit to eginhard/radian that referenced this pull request Dec 9, 2023
The typescript language server `ts-ls` crashes on start-up because of a
deprecated argument, which was fixed in
emacs-lsp/lsp-mode#4202

See also doomemacs/doomemacs#7540
ncaq added a commit to ncaq/lsp-mode that referenced this pull request 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)
@apoorv569 apoorv569 mentioned this pull request Apr 4, 2024
jcs090218 pushed a commit that referenced this pull request 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 pull request 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
client One or more of lsp-mode language clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants