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

Feature: node executable name (closes #232 #237) #302

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

thodnev
Copy link

@thodnev thodnev commented Oct 14, 2024

Allows providing tsserver runtime names other than node, i.e bun (for improved perfomance), etc.
Closes #232 #237
Contains fixes to summarized proposals by the others in #232.
If someone has access to Windows machine, could you test it once again and we're good to go

@thodnev thodnev force-pushed the feature/node-executable branch from 9a9b1db to 9530703 Compare October 14, 2024 23:39
@qwexvf
Copy link

qwexvf commented Oct 16, 2024

@thodnev hey thanks for the pr!
i was testing this and noticed that it requires npm to execute the tsserver do you think its possible to use bun in this case too?

.../.local/share/nvim/lazy/plenary.nvim/lua/plenary/job.lua:108: npm: Executable not found
stack traceback:
        .../.local/share/nvim/lazy/plenary.nvim/lua/plenary/job.lua:108: in function 'new'
        ...pt-tools.nvim/lua/typescript-tools/tsserver_provider.lua:76: in function 'new'
        ...pt-tools.nvim/lua/typescript-tools/tsserver_provider.lua:105: in function 'init'
        .../typescript-tools.nvim/lua/typescript-tools/tsserver.lua:38: in function 'new'
        .../lazy/typescript-tools.nvim/lua/typescript-tools/rpc.lua:18: in function 'config_cmd'
        /usr/share/nvim/runtime/lua/vim/lsp/client.lua:510: in function </usr/share/nvim/runtime/lua/vim/lsp/client.lua:442>
        [C]: in function 'pcall'
        /usr/share/nvim/runtime/lua/vim/lsp.lua:443: in function 'start_client'
        ...share/nvim/lazy/nvim-lspconfig/lua/lspconfig/manager.lua:154: in function 'add'
        ...share/nvim/lazy/nvim-lspconfig/lua/lspconfig/manager.lua:284: in function <...share/nvim/lazy/nvim-lspconfig/lua/lspconfig/manager.lua:271>
        [C]: in function 'pcall'
        ...l/share/nvim/lazy/nvim-lspconfig/lua/lspconfig/async.lua:5: in function <...l/share/nvim/lazy/nvim-lspconfig/lua/lspconfig/async.lua:4>

@thodnev
Copy link
Author

thodnev commented Oct 16, 2024

@qwexvf Thanks!
Yes, on my machine nvim starts bun as expected. Here is a part of my nvim config:

-- init.lua

-- Plugins used by lazy.nvim
local plugins = {
    -- ...
    -- Local plugin
    {dir = vim.fn.stdpath('config') .. '/typescript-tools.nvim',
     dependencies = {'nvim-lua/plenary.nvim', 'neovim/nvim-lspconfig'},
     opts = {
         settings = {
             tsserver_node_executable = 'bun'
         }
     }}
}

The other problem is that bun itself (unless forced to do otherwise with --bun flag) should shebang inside a tsserver script and will run node, so by default the performance gain will be only for the startup time (as per this post).

BUT. Even with --bun flag, right now for some strange reason bun still starts nodejs, and running bun run --bun tsserver, my ps axjf | grep -B5 tsserver shows hierarchy:

   2269  503521  503521  503521 ?             -1 Ssl   1000   0:01  \_ /usr/bin/kitty
 503521  503534  503534  503534 pts/1     508102 Ss    1000   0:00  |   \_ /bin/bash --posix
 503534  508102  508102  503534 pts/1     508102 S+    1000   0:00  |       \_ bun run --bun tsserver
 508102  508103  508102  503534 pts/1     508102 Sl+   1000   0:00  |           \_ node /home/thd/Work/StrongerFox/node_modules/.bin/tsserver

It happens unless I patch the tsserver script changing shebang to bun manually:

diff -u a/tsserver b/tsserver
--- a/tsserver
+++ b/tsserver
@@ -1,2 +1,2 @@
-#!/usr/bin/env node
+#!/usr/bin/env bun
 require('../lib/tsserver.js')

Then finally I get bun working with all it brewed properly, and a proper process tree:

   2269  503521  503521  503521 ?             -1 Ssl   1000   0:01  \_ /usr/bin/kitty
 503521  503534  503534  503534 pts/1     516931 Ss    1000   0:00  |   \_ /bin/bash --posix
 503534  516931  516931  503534 pts/1     516931 S+    1000   0:00  |       \_ nvim ./repmax.ts
 516931  516932  516932  516932 ?             -1 Ss    1000   0:00  |           \_ nvim --embed ./repmax.ts
 516932  516964  516964  516964 ?             -1 Ssl   1000   0:02  |               \_ bun /home/thd/Work/StrongerFox/node_modules/typescript/lib/tsserver.js --stdio --locale en --useInferredProjectPerProjectRoot --validateDefaultNpmLocation --noGetErrOnBackgroundUpdate --cancellationPipeName /tmp/tsserver_nvim_cj3r6d/seq_*

We have nothing to do with these bun & typescript policies. A proper support without patching will be added sooner or later.
I will have to look into other possible solutions for bun

@qwexvf
Copy link

qwexvf commented Oct 18, 2024

@thodnev thanks for the detailed response and i can confirm that your method above works perfectly!
my question was the plugin will fail if i don't have npm installed on my machine. is it possible to make it use bun ?
i'm also going to look into this cause i don't know what they are using npm for 🤔

@thodnev
Copy link
Author

thodnev commented Oct 18, 2024

@qwexvf That is great news!
Unfortunately, the usual "build from source" option will not work with bun. As building bun from source currently requires having bun installed for some of their transformation scripts to work.

But it can be installed with npm if it is present on target system. And global installs will not pollute the system dirs, as bun is currently distributed as just one executable with all the batteries included. So npm install -g bun should work fine.

Another option would be to run their official install script. I haven't looked much into it, as running directly downloaded bash scripts is not everyone likes doing. But their script does not require running with root privileges, so probably it is not that big deal

And running these commands could be packed into nvim bootstrap, for example, like lazy does here

Another good option for future could be adding bun to mason.nvim, as a more nvim-native tooling solution. I have looked through Mason package list, unfortunately neither bun nor npm aren't provided there yet

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

Successfully merging this pull request may close these issues.

Make node executable configurable
3 participants