Allow use of electron runtime for Node-based servers

[rchl]

Since electron builds have pointer compression enabled, those use quite a bit less memory than the official Node.js builds.

I haven't been able to find official Node.js builds with pointer compression enabled so using Electron itself is the next best thing and is easiest to handle since those can be installed as a NPM dependency.

Electron binary is fairly big itself (about 200MB on Mac, compared to about 100MB for Node.js) since it contains fair amount of extra stuff besides just Node.js build but it's not relevant for memory usage at runtime.

For reference, on Mac, the LSP-typescript running on a VSCode repo with a bunch of files opened uses following amounts of memory:

• 210MB when running on the official Node.js build
• 142MB when running on Electron's Node.js build

[predragnikolic]

I haven't tired this,(and I wont be able to test this this weekend) But +1 for this,

I would rather download 100MB more ONCE, and use less RAM all the time. than to download 100MB less ONCE, and consume more RAM all the time.

I just wasn't sure if an additional setting was necessary, I wanted to purpose to add a new node_runtime array value, "local_electron", but an additional setting, is also fine by me.

[predragnikolic]

Second, Not related to this PR. Is having this many node_runtime options necessary? Would it be easier if we removed the node_runtime and use_electron_for_local_runtime setting and only have one runtime. That is the electron node one (or the local node) or whatever makes more sense.

The reason why I am saying this, is because the system node_runtime... while useful, is not that great when using with nvm (Node Version Manager). I use nvm all the time and by accident I had "system" specified in the lsp_utils setting. I changed the node version a couple of times with nvm and after ~one month ended up with many node folders inside LSP-* plugins in Package Storage that I had to manually delete.

λ  ~/.cache/sublime-text/Package Storage/LSP-typescript  tree -L 2
.
├── 12.20.2
│   └── typescript-language-server
...
├── 16.15.0
│   └── typescript-language-server
└── 16.16.0
    └── typescript-language-server

Most users are not aware of this and I consider this not ideal.

Either way, I would not change the current behavior. I just said this to share my thoughts :)

[rchl]

Is having this many node_runtime options necessary? Would it be easier if we removed the node_runtime and use_electron_for_local_runtime setting and only have one runtime. That is the electron node one (or the local node) or whatever makes more sense.

I suppose the approach I've chosen here is to not force anyone into downloading extra stuff since more advanced users might not like that. So both local runtime and now electron are opt-ins.

It's a choice that was made at the beginning and I don't think we should change it now.

The situation with multiple versions could be seen as a separate issue. One that already exists.

[predragnikolic]

Looks like there are some issues on Linux.

currently I have set:

// Settings in here override those in "lsp_utils/lsp_utils.sublime-settings"
{
    "nodejs_runtime": ["local"],
    "use_electron_for_local_runtime": true
}

When I start sublime again In the status bar I see "Downloading Electron"

Once downloaded, I get this error for every language server:

Here are the logs

``` reloading python 3.3 plugin lsp_maintainers.main reloading python 3.3 plugin repository.auto-update-repository reloading python 3.3 plugin Terminal.Terminal reloading python 3.3 plugin UnitTesting33.ut plugins loaded setup [lsp_utils] Installing Electron dependency... reloading python 3.3 plugin AutomaticPackageReloader33.package_reloader reloading settings Packages/User/Preferences.sublime-settings dictionary Packages/Language - English/en_US.dic not found Errors parsing theme: "class" is required in Packages/Sone/Sone-Adaptive.sublime-theme:285:9 Package Control: Skipping automatic upgrade, last run at 2023-02-28 22:56:59, next run at 2023-02-28 23:56:59 or after [lsp_utils] START output of command: "install-gelectron@22.2.0" changed 71 packages, and audited 72 packages in 8s 17 packages are looking for funding run `npm fund` for details found 0 vulnerabilities [lsp_utils] Command output END Unable to start subprocess for LSP-typescript Traceback (most recent call last): File "/home/predragnikolic/.config/sublime-text/Packages/LSP/plugin/core/windows.py", line 242, in start_async additional_variables = plugin_class.additional_variables() File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/_client_handler/abstract_plugin.py", line 96, in additional_variables return cls.get_additional_variables() File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/npm_client_handler.py", line 91, in get_additional_variables 'node_bin': cls._node_bin(), File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/npm_client_handler.py", line 156, in _node_bin return cls.__server.node_bin File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/server_npm_resource.py", line 62, in node_bin raise Exception('Failed to resolve path to the Node.js runtime') Exception: Failed to resolve path to the Node.js runtime Unable to start subprocess for LSP-cspell Traceback (most recent call last): File "/home/predragnikolic/.config/sublime-text/Packages/LSP/plugin/core/windows.py", line 242, in start_async additional_variables = plugin_class.additional_variables() File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/_client_handler/abstract_plugin.py", line 96, in additional_variables return cls.get_additional_variables() File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/npm_client_handler.py", line 91, in get_additional_variables 'node_bin': cls._node_bin(), File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/npm_client_handler.py", line 156, in _node_bin return cls.__server.node_bin File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/server_npm_resource.py", line 62, in node_bin raise Exception('Failed to resolve path to the Node.js runtime') Exception: Failed to resolve path to the Node.js runtime Unable to start subprocess for LSP-tailwindcss Traceback (most recent call last): File "/home/predragnikolic/.config/sublime-text/Packages/LSP/plugin/core/windows.py", line 242, in start_async additional_variables = plugin_class.additional_variables() File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/_client_handler/abstract_plugin.py", line 96, in additional_variables return cls.get_additional_variables() File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/npm_client_handler.py", line 91, in get_additional_variables 'node_bin': cls._node_bin(), File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/npm_client_handler.py", line 156, in _node_bin return cls.__server.node_bin File "/home/predragnikolic/.config/sublime-text/Packages/lsp_utils/st3/lsp_utils/server_npm_resource.py", line 62, in node_bin raise Exception('Failed to resolve path to the Node.js runtime') Exception: Failed to resolve path to the Node.js runtime ```

[predragnikolic]

FWIW here is the contents in Pacakge Storage.

  ~/.cache/sublime-text/Package Storage/lsp_utils  tree -L 2        
.
└── node-runtime
    └── 16.17.1

[rchl]

Probably the path to the electron binary is different on linux then Mac and Windows. On mac it's in Package Storage/lsp_utils/node-runtime/16.17.1/node/lib/node_modules/electron/dist/Electron.app/....

For Linux I'm assuming it's in Package Storage/lsp_utils/node-runtime/16.17.1/node/lib/node_modules/electron/dist/electron but I guess it might be wrong.

On error the installation is deleted so it's hard to debug I guess. You might want to just do npm i electron in any project and check where the binary ends up in node_modules.

[rchl]

Seems to work just fine for me on Debian 11 ARM.

[jfcherng]

Just tried and this is such a awesome memory saver :)

[predragnikolic]

For Linux I'm assuming it's in Package Storage/lsp_utils/node-runtime/16.17.1/node/lib/node_modules/electron/dist/electron but I guess it might be wrong. On error the installation is deleted so it's hard to debug I guess. You might want to just do npm i electron in any project and check where the binary ends up in node_modules.

I installed electron just to verify the path and the path looks correct node_modules/electron/dist/electron

Not sure why I get errors, will try to look into it when I can. I am on Ubuntu 22.10

[rchl]

Based on your logs, electron was actually installed correctly so the binary should be there. You confused me by only listing 2 levels deep.

Don't see why it would fail to resolve path. Have you tried restarting? :)

[predragnikolic]

Give me some time :)

[predragnikolic]

Ok, now I have reasons on why it fails on my Laptop.

I changed my default npm install directory to be ~/.npm-global. (see https://docs.npmjs.com/resolving-eacces-permissions-errors-when-installing-packages-globally) I did this to avoid using sudo when installing npm pacakges globally.

This function will return None but I expected to return a path to electron:

def _resolve_electron_binary(self) -> Optional[str]:
        if not self._use_electron:
            return None
        electron_dist_dir = path.join(self._resolve_lib(), 'electron', 'dist')
        binary_path = None
        platform = sublime.platform()
        if platform == 'osx':
            binary_path = path.join(electron_dist_dir, 'Electron.app', 'Contents', 'MacOS', 'Electron')
        elif platform == 'windows':
            binary_path = path.join(electron_dist_dir, 'electron.exe')
        else:
            binary_path = path.join(electron_dist_dir, 'electron')
        return binary_path if binary_path and path.isfile(binary_path) else None

The binary_path variable is equal to /home/predragnikolic/.cache/sublime-text/Package Storage/lsp_utils/node-runtime/16.17.1/node/lib/node_modules/electron/dist/electron but that path doesn't exist on my laptop.

I only have 2 folders in .../lib/node_modules/:

λ  ~/.cache/sublime-text/Package Storage/lsp_utils/node-runtime/16.17.1/node/lib/node_modules  ls
corepack  npm

Lets take a step back and see how electron is installed:

Electron is installed with this code:

self.run_npm(['install', '-g', 'electron@{}'.format(ELECTRON_VERSION)], cwd=self._node_dir)

which is basally this command npm -g electron@22.2.0

and because I set a custom npm directory, it is acutally installed at this location inside .npm-global:

λ  ~/.npm-global/lib/node_modules  tree -L 1
.
├── @angular
├── browser-sync
 ...
├── electron  // Here it is
 ...
└── yarn

[predragnikolic]

I managed to fix it for myself by using the --prefix flag to specify the install directory,

[rchl]

Isn't it a bad idea to have a global configuration like that if you are switching between different versions of node? That means that all would share the same cache which I imagine is not ideal.

But I will look into it more. I think the best solution might be to ignore user settings completely instead of just overriding prefix but I'd have to investigate more.

[rchl]

I'm now ignoring user's config.

But I'm thinking it might be problematic if someone NEEDS to, for example, use some proxy to download npm packages...

I suppose lsp_utils could support specifying custom NPM options so that user could set those individually.

[rchl]

Also added support for specifying custom npm config overrides.

[predragnikolic]

The last changes fixed the issue

Isn't it a bad idea to have a global configuration like that if you are switching between different versions of node?

Turns out nvm doesn't support the prefix option.

I found this article which helped me better understand things https://sebhastian.com/nvm-prefix-issue/

Most of the time, the prefix option is set to avoid permission issues blocking the npm install command. When using NVM, the prefix is not needed because NVM installs a global package in a folder that doesn’t require superuser permissions.

[rchl]

I'm struggling with deciding whether electron should be used by default.

If I don't make it the default then most people will not even know that it's an option and won't benefit from lower memory usage.

If I make it the default then it will trigger additional 200MB download which could make some people unhappy, I imagine.

There is also an option of asking using a dialog but would that be a separate dialog from the existing "Download Node.js" one or just merged into the existing one? Triggering two dialogs is not what I'd call a very friendly UX.

[jfcherng]

How about using a file under Package Storage/lsp_utils/ to flag the user has made "Node.js or Electron" choice?

• If the flag file doesn't exist and the user uses "local", we ask "Node.js or Electron" and create the flag file.
• If the flag file exists, just don't bother the user.

[rchl]

It would all just complicate things with more buttons. We would have to make text longer to explain the difference so that the user could make an informed decision.

[LDAP]

The only downside of electron are the 100mb additional download? I think we can even drop node completely. If advanced users want to use node they may want to manage it themselves anyway?

[jfcherng]

Unfortunately, we still need npm-cli. Unless we can manage to use Electron to run standalone(?) npm-cli.

Or maybe it's simpler to use yarn instead? It looks like yarn is just a single 4.87 MB .js file or 1.19 MB tarball ( https://github.com/yarnpkg/yarn/releases/tag/v1.22.19 ) so it's so easy to use Electron to execute it. Not sure whether yarn is still faster than npm at this moment haha.

Other than that, Electron can be downloaded from https://github.com/electron/electron/releases/tag/v22.3.2 directly.

[rchl]

All our helper packages use package-lock.json. I don't think yarn would respect that so we'd have to update all to use yarn then?

[jfcherng]

All our helper packages use package-lock.json. I don't think yarn would respect that so we'd have to update all to use yarn then?

Consider that npm-cli is sort of a builtin in Node.js, we may still maintain package-lock.json. But when lsp_utils does package installation, maybe

1. $ yarn import to convert package-lock.json into yarn.lock ( https://classic.yarnpkg.com/blog/2018/06/04/yarn-import-package-lock/ )
2. The previous step generates node_modules/.yarn-integrity, which seems to prevent from $ yarn install, and thus we delete it.
3. $ yarn install

but this sounds quite experimental.

[rchl]

I've managed to make it work with manually managed electron and using downloaded yarn but I'm concerned a bit because it wouldn't give people choice to use normal Node anymore. I wonder whether it would be problematic for some. Maybe some companies are more strict about using Electron?

[rchl]

Actually it's problematic to manually manage Electron using github as archives are in zip format and Python's zipfile.ZipFile removes executable bit from all binaries. And there are many binaries in the archive (on Mac at least) so it would be a pain to manually update those...

[rchl]

Worked around executable bit issue with help of https://stackoverflow.com/questions/39296101/python-zipfile-removes-execute-permissions-from-binaries but MacOS still doesn't let it run. It appears that there are differences in how symlinks are treated. In the non working one there are no symlinks...

[jfcherng]

Or on MacOS, maybe we call external program to do unzip? Is unzip a builtin on Mac?

[rchl]

Yeah, could go that route probably.

But all that would make the changes a lot more significant and risky. I'm not convinced that I would want to release that as enabled by default when using that approach.

Now I think I should implement that but as an option and only enable by default once it was tested manually by volunteers.

[jfcherng]

But all that would make the changes a lot more significant and risky.

Yeah, it sounds like there are too many "if" branch to implement.

[rchl]

OK, now there are two separate runtimes so it's less risky. Only some minor changes were done to the existing Node runtime and its version wasn't changed to avoid extra friction.

I think we can ship this as is now.