Update default minimum Node.js version to 16.0.0 #19192

kripken · 2023-04-17T16:45:47Z

This version of node supports wasm-bigint by default, so it will help #19156

This is about 2 years old, and the node website already provides newer versions
both for latest and for LTS. However, other places are behind, for example the
latest Ubuntu LTS (22) has Node 12, so perhaps we should wait on this.

This is possible after emscripten-core/emsdk#829 made
the emsdk install a 15.x version by default, and then
emscripten-core/emsdk#1232 switched to 16.

This is about 2 years old, and the node website already provides newer versions both for latest and for LTS. However, other places are behind, for example the latest Ubuntu LTS (22) has Node 12, so perhaps we should wait on this. This is possible after emscripten-core/emsdk#829 made the emsdk install a 15.x version by default.

sbc100

lgtm % discussions about when to actually land this

jeswr · 2023-05-05T19:54:50Z

Node 15 no longer has active or security support. Why not go straight to Node 16 which is currently in maintenance LTS?

kripken · 2023-05-05T20:17:46Z

@jeswr Good point, yeah, it might be better to do that. This came up on the mailing list too:

https://groups.google.com/g/emscripten-discuss/c/HmCTeDEw6cQ/m/-nYu5HqfCAAJ

It does seem more obvious to go for 16, but that does allow strictly fewer installations than going to 15 (and 15 is the minimum we need for the wasm-bigint feature), so it's not obvious to me what is best.

jeswr · 2023-05-05T20:36:07Z

so it's not obvious to me what is best.

I'd be suggesting 16. Odd versions of node (including 15) are short lived by design and shouldn't really end up being used in any production scenarios - Node 15 has been deprecated for almost 2 years now (c.f. https://endoflife.date/nodejs).

This is possible after emscripten-core/emsdk#829 made
the emsdk install a 15.x version by default.

I'd be suggesting to upgrade emsdk to 16.x as well (and always using even versions of Node as defaults going into the future).

jeswr · 2023-05-05T20:47:21Z

Just read the mailing list as well and wanted to respond to a particular comment there.

Since emsdk (2) has already been updated, I'm not sure there is any reason to update again from 15.14.0 to 16.0.0. I'm not sure what benefit that would have.

This would actually be quite a beneficial update in our use case in swipl-wasm. I currently have a draft PR open where we are needing to switch over to node 16 for the build step in order to generate a file that will pass our ctest test suite. Using Node 15 to build some of our tests result in UnhandledRejectionErrors. I have not dug into what the root cause of these errors were when building with Node 15.

sbc100 · 2023-05-08T20:21:26Z

Just read the mailing list as well and wanted to respond to a particular comment there.

Since emsdk (2) has already been updated, I'm not sure there is any reason to update again from 15.14.0 to 16.0.0. I'm not sure what benefit that would have.

This would actually be quite a beneficial update in our use case in swipl-wasm. I currently have a draft PR open where we are needing to switch over to node 16 for the build step in order to generate a file that will pass our ctest test suite. Using Node 15 to build some of our tests result in UnhandledRejectionErrors. I have not dug into what the root cause of these errors were when building with Node 15.

Are the errors you are seeing from running the tests (i.e. running the output of emscripten)?

Remember this issue is just about the minimum node version that we target by default. You are still free to run the output on a more recent version, such as 16.. or whatever you want.

sbc100 · 2023-05-08T20:25:00Z

If there is an issue with UnhandledRejectionError when running on node 15 we should fix that regardless of the default we target since users can always target an explictly older version. MIN_NODE_VERSION can be set to low as 10.19.0, and we do test the output and run some tests with that ancient version of node.

sbc100 · 2023-05-08T20:27:05Z

If there is an issue with UnhandledRejectionError when running on node 15 we should fix that regardless of the default we target since users can always target an explictly older version. MIN_NODE_VERSION can be set to low as 10.19.0, and we do test the output and run some tests with that ancient version of node.

hoodmane · 2023-05-09T15:27:33Z

I would also like to see emsdk stop placing node on the path automatically. Or at least not at the start of the path, I would be okay with adding node to the end of the path. Particularly if there is already a newer node around that I would prefer to use it is annoying to have to adjust the path to work around the older node.

sbc100 · 2023-05-09T16:00:48Z

I would also like to see emsdk stop placing node on the path automatically. Or at least not at the start of the path, I would be okay with adding node to the end of the path. Particularly if there is already a newer node around that I would prefer to use it is annoying to have to adjust the path to work around the older node.

This was largely fixed in #1189/#1209 I believe.

kripken · 2023-05-09T20:38:14Z

After thinking more on this I am convinced that we should switch to 16 by default. It is less surprising by far - we've had several people mention that - and also this is tied to the version the emsdk installs by default, and 16 is just much more useful for people to have (since it is a supported version that people might also find useful in their CIs etc.).

But that would depend on making the emsdk install 16 by default. @sbc100 , I know you did the upgrade to 15 fairly recently, and I guess there's no reason to rush to do it again. But next time we think about an update there, I think it would be good to do. In general, I think it makes sense for us to eventually switch to having the emsdk install node LTS versions by default.

sbc100 · 2023-06-14T16:52:51Z

Looks like node 16 is coming to emsdk: emscripten-core/emsdk#1232

kripken · 2023-06-20T19:04:21Z

Updated to Node 16 after emscripten-core/emsdk#1232 landed.

sbc100 · 2023-06-20T19:06:11Z

ChangeLog.md

@@ -20,6 +20,8 @@ See docs/process.md for more on how version tagging works.

 3.1.42 (in development)
 -----------------------
+- Bump the default minimum Node version from 10.19 to 16.0. To target the


The default minimum Node version was bumped

Also can you add the word "default" to the PR title?

Done, but isn't "was bumped" passive voice, which is generally frowned upon in English? I guess just "bump" as in the old text isn't even grammatical...

"Bump the default minimum Node version" sounds correct grammatically but that sounds like that text I would write for the PR itself. i.e. its in the present tense, imperative mood. e.d. "Rename foo", "Delete X"

For the changelog entry I normally write in the past tense think. e.g. "foo was renamed", "x was deleted". So I would expect "was bumped" here... but I could be wrong. Reading the rest of the changelog it seems to be mostly be in the past. Should we swith to using the same tense/mood for PR and for the changelog?

I'm not sure. Let's go with your intuition here (changelog in the past, PRs in the present).

kripken · 2023-06-20T20:23:53Z

A code size test improved here. While improving it I also made it work with --rebaseline.

kripken · 2023-06-20T20:26:16Z

The code size improvement is due to this diff:

64,69d63
<  var nodeMajor = process.versions.node.split(".")[0];
<  if (nodeMajor < 15) {
<   process.on("unhandledRejection", reason => {
<    throw reason;
<   });
<  }

sbc100 · 2023-06-21T14:41:50Z

15.x -> 16.x in the PR description.

sbc100 · 2023-06-21T14:42:43Z

15.x -> 16.x in the PR description.

And #1232 is the emsdk PR that made it possible

…self.emcc_args which are populated with the node flags

kripken · 2023-06-21T15:51:39Z

This also required a test fix, to make a test use self.build(). That way the test uses self.emcc_args which we populate with the right node flags for emcc automatically.

sbc100 · 2023-06-21T15:53:00Z

lgtm! Once this landed we should do a release I think.

kripken · 2023-06-21T17:53:39Z

Ok, I think this is ready to land. We've collected feedback both here and on the mailing list and there were no concerns, aside from actually suggesting to go to 16 and not just 15, as we then agreed.

kripken requested a review from sbc100 April 17, 2023 16:45

sbc100 approved these changes Apr 17, 2023

View reviewed changes

kripken added 2 commits April 17, 2023 16:22

Merge remote-tracking branch 'origin/main' into min.node.15

5712fff

changlog [ci skip]

62af277

kripken added 2 commits June 20, 2023 12:02

Merge remote-tracking branch 'origin/main' into min.node.15

501f34e

16

3f9591d

sbc100 reviewed Jun 20, 2023

View reviewed changes

sbc100 approved these changes Jun 20, 2023

View reviewed changes

update and generalize test

d95cc1e

kripken changed the title ~~Update minimum Node.js version to 15.0.0~~ Update default minimum Node.js version to 15.0.0 Jun 20, 2023

kripken changed the title ~~Update default minimum Node.js version to 15.0.0~~ Update default minimum Node.js version to 16.0.0 Jun 20, 2023

feedback

a03ff65

Merge branch 'main' into min.node.15

b1fc768

sbc100 approved these changes Jun 21, 2023

View reviewed changes

kripken added 5 commits June 21, 2023 08:43

Merge remote-tracking branch 'origin/main' into min.node.15

431c208

Make other.test_node_unhandled_rejection use self.build() so it uses …

4c2a6e6

…self.emcc_args which are populated with the node flags

Merge remote-tracking branch 'origin/min.node.15' into min.node.15

bf6fa36

clarify changelog

b2cb9c7

clarify changelog

fcbb74c

sbc100 approved these changes Jun 21, 2023

View reviewed changes

kripken merged commit df1f6f7 into main Jun 21, 2023

kripken deleted the min.node.15 branch June 21, 2023 17:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update default minimum Node.js version to 16.0.0 #19192

Update default minimum Node.js version to 16.0.0 #19192

kripken commented Apr 17, 2023 •

edited

Loading

sbc100 left a comment

jeswr commented May 5, 2023

kripken commented May 5, 2023 •

edited

Loading

jeswr commented May 5, 2023 •

edited

Loading

jeswr commented May 5, 2023 •

edited

Loading

sbc100 commented May 8, 2023

sbc100 commented May 8, 2023

sbc100 commented May 8, 2023

hoodmane commented May 9, 2023

sbc100 commented May 9, 2023 •

edited

Loading

kripken commented May 9, 2023

sbc100 commented Jun 14, 2023

kripken commented Jun 20, 2023

sbc100 Jun 20, 2023

kripken Jun 20, 2023

sbc100 Jun 20, 2023

kripken Jun 20, 2023

kripken commented Jun 20, 2023

kripken commented Jun 20, 2023

sbc100 commented Jun 21, 2023

sbc100 commented Jun 21, 2023

kripken commented Jun 21, 2023

sbc100 commented Jun 21, 2023

kripken commented Jun 21, 2023

Update default minimum Node.js version to 16.0.0 #19192

Update default minimum Node.js version to 16.0.0 #19192

Conversation

kripken commented Apr 17, 2023 • edited Loading

sbc100 left a comment

Choose a reason for hiding this comment

jeswr commented May 5, 2023

kripken commented May 5, 2023 • edited Loading

jeswr commented May 5, 2023 • edited Loading

jeswr commented May 5, 2023 • edited Loading

sbc100 commented May 8, 2023

sbc100 commented May 8, 2023

sbc100 commented May 8, 2023

hoodmane commented May 9, 2023

sbc100 commented May 9, 2023 • edited Loading

kripken commented May 9, 2023

sbc100 commented Jun 14, 2023

kripken commented Jun 20, 2023

sbc100 Jun 20, 2023

Choose a reason for hiding this comment

kripken Jun 20, 2023

Choose a reason for hiding this comment

sbc100 Jun 20, 2023

Choose a reason for hiding this comment

kripken Jun 20, 2023

Choose a reason for hiding this comment

kripken commented Jun 20, 2023

kripken commented Jun 20, 2023

sbc100 commented Jun 21, 2023

sbc100 commented Jun 21, 2023

kripken commented Jun 21, 2023

sbc100 commented Jun 21, 2023

kripken commented Jun 21, 2023

kripken commented Apr 17, 2023 •

edited

Loading

kripken commented May 5, 2023 •

edited

Loading

jeswr commented May 5, 2023 •

edited

Loading

jeswr commented May 5, 2023 •

edited

Loading

sbc100 commented May 9, 2023 •

edited

Loading