GH-7349: Support for Node.js 12.x

[kittaakos]

Closes #7349

Signed-off-by: Akos Kitta [email protected]

What it does

With this PR you can build Theia with Node.js 12.x.

How to test

This is a good question. We build it on both Node.js 10.x and 12.x on the CI. Any further ideas?

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[kittaakos]

I leave this decision to the community. We can support the latest electron version too: #7968

[akosyakov]

This is a good question. We build it on both Node.js 10.x and 12.x on the CI. Any further ideas?

We smoke test languages, git, file watching, keyboard detection and so on for browser and electron targets. Especially for places where we make use of native Node modules. The same for Electron 9 PR.

If we agree to go for Electron 9 I would prefer to have one PR which bumps up Node and Electron and we test it only once.

[kittaakos]

If we agree to go for Electron 9 I would prefer to have one PR which bumps up Node and Electron and we test it only once.

Sure. Let's wait for other's feedback on this and I am happy to make the changes if we decide to move forward.

[benoitf]

I think it's great if we have support for nodejs 10 and 12 for a while, so downstream can update to 12 as well and then like one release later, nodejs 10 support can be removed with Electron 9 requirement.

[akosyakov]

I think it's great if we have support for nodejs 10 and 12 for a while, so downstream can update to 12 as well and then like one release later, nodejs 10 support can be removed with Electron 9 requirement.

Sounds reasonable If noone needs new Electron urgently and can wait till August. Added to next dev meeting.

In anyway it would be good to merge this PR rather soon, if we want to have it in next release. Also adding a notice in CHANGELOG that Node.js 10 support is going to be dropped after 1.3.0 and notifying about it on Spectrum after merging the PR.

[kittaakos]

notice in CHANGELOG that Node.js 10 support is going to be dropped

Why does it mean we have to drop Node.js 10.x? Can someone please explain this to me? 😊What's the problem here? Is it that we might introduce something new that is available only with Node.js 12.x API?

[akosyakov]

Why does it mean we have to drop Node.js 10.x?

• Because Electron 9 requires Node.js 12.14 as the minimal version and we use Electron Node version as our baseline.
• Node.js 10.x is not maintained since April 2021, we need to move our baseline to the active LTS: https://nodejs.org/en/about/releases/
• VS Code uses Node.js 12 as the baseline and it can cause issues with porting code and running VS Code extensions using features of Node.js 12

[paul-marechal]

Why does it mean we have to drop Node.js 10.x? [...] Is it that we might introduce something new that is available only with Node.js 12.x API?

Yes. So if we start using those new features/apis someone on Node 10 won't be able to run it. I remember we had a similar issue with some missing Object.entries in some runtime but I don't remember exactly.

[thegecko]

I remember we had a similar issue...

Yes we did, I'd rather we avoided that again. If we have a strong reason to support versions outside of the electron version being supported, we need to have a strong CI in place!

That being said an upgrade is fine if we can do electron at the same time IMO :)

[kittaakos]

remember we had a similar issue with some missing Object.entries in some runtime

I found this and this. Correct, it was a Node.js incompatibility issue, but the other way around, the electron version was outdated (8.x). We have already switched to Node.js 10.x for the development and used new APIs. It failed under electron (8.x); the solution was to update the electron version (10.x). Correct me if I am wrong.
I do not understand why couldn't we support both versions for one or two releases. Let's stick to the 10.x API (with the typings) for the development, but we can already support electron 9.x (12.x), can't we? 😊

I am closing this PR in favor of #7968. I want to merge the updates into one. Let's move the conversation there. Thank you!

[akosyakov]

@kittaakos What you are saying is fine. But we should notify adopters in advance that we are going to drop Node 10 support in 1-2 releases. I am not sure what is the proper channel. I also don't think that's a really big deal, Node adopters have to take care this year anyway to get rid of using Node 10. I've checked with Gitpod, the only component which rely on Node 10 is Theia, everything else was already moved to the active LTS. We have issue in theia-apps to use Node 12 as well: theia-ide/theia-apps#358 (comment)

[kittaakos]

But we should notify adopters in advance that we are going to drop Node 10 support in 1-2 releases.

Sure, I totally agree.

I am not sure what is the proper channel.

Spectrum, GH, npm log (if <12.14.1), etc.. 😄