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

feat: update local nodejs and electron runtimes to v18 #103

Merged
merged 2 commits into from
Oct 14, 2023
Merged

feat: update local nodejs and electron runtimes to v18 #103

merged 2 commits into from
Oct 14, 2023

Conversation

deathaxe
Copy link
Contributor

This PR...

  1. updates NodeJS to 18.18.1
  2. updates Electron to 26.4.0

Comment on lines 28 to 29
ELECTRON_RUNTIME_VERSION = '26.4.0' # includes Node.js v18.16.1
ELECTRON_NODE_VERSION = '18.16.1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also https://github.com/electron/electron/releases/tag/v27.0.0 which includes Node 18.17.1.

Note that I'm thinking of introducing some cleanup code for old versions so I might want to introduce all those changes together. Though I'm not sure how I'll do it yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup code might be something for both, the runtime handler and NpmClientHandler

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would need to be somewhat lazy as upon update older releases may still be running and therefore locked on Windows, so removing immediately might fail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Package updates typically happen on startup and this cleanup would happen before servers are started so it wouldn't be an issue.

It could be an issue when triggering package update manually but then that already can be problematic so I wouldn't worry too much about that. It should correct itself on next start.

@jfcherng
Copy link
Contributor

jfcherng commented Oct 13, 2023

node.js v20 LTS is on its way by the end of this month btw. but there is no electron using it at this moment.

@rchl rchl changed the title Update local nodejs and electron runtimes feat: update local nodejs and electron runtimes to v18 Oct 14, 2023
@rchl rchl merged commit 2ef8712 into sublimelsp:master Oct 14, 2023
3 of 5 checks passed
@deathaxe deathaxe deleted the pr/update-node-and-electron branch October 14, 2023 21:13
@rchl
Copy link
Member

rchl commented Oct 16, 2023

node.js v20 LTS is on its way by the end of this month btw. but there is no electron using it at this moment.

But I suppose it will take Electron quite a while to update to that.
Also, I would not want to do frequent updates of node runtime unless absolutely necessary so I'm fine with just sticking to 18 which should get only absolutely necessary updates (like security updates).

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.

3 participants