fix(terser): always make at least 1 worker thread

[easrng]

Rollup Plugin Name: terser

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:
Resolves #1594

Description

On some platforms (I noticed it on Android), os.cpus() can return [], resulting in no workers being spawned, so the jobs never run. This patch makes sure the maximum number of workers is never 0.

[dasa]

A future enhancement could be to use os.availableParallelism (requires v18.14+), since it's recommended over using os.cpus().length.

[shellscape]

Looks like you need an upstream update:

git remote add upstream git@github.com:rollup/plugins.git
git fetch upstream
git merge upstream/master
git push

That should get you set so that Windows Node 20 action passes.

[shellscape]

Alright I updated your fork for you from upstream/master and looks like there's a failure on Windows directly related to WASM. Please have a look.

[dasa]

Maybe the test needs to also make os.availableParallelism unavailable, otherwise it won't use os.cpus.

[easrng]

Alright I updated your fork for you from upstream/master and looks like there's a failure on Windows directly related to WASM. Please have a look.

Aren't V8 errors either bugs in node itself or OOMs usually?

[shellscape]

Not sure in this case. It doesn't happen on the Ubuntu builds. The existing code doesn't throw this error, so it's possible the changes have surfaced a bug.

[dasa]

Maybe overwriting os.cpus in this way is breaking Node 20.

[shellscape]

@easrng could you please take a look?

[dasa]

@easrng should this also include os.availableParallelism = null;?

[dasa]

Maybe this change is worth a try?

diff --git a/packages/terser/test/test.js b/packages/terser/test/test.js
index 9dabacd..fd36d58 100644
--- a/packages/terser/test/test.js
+++ b/packages/terser/test/test.js
@@ -360,7 +360,9 @@ test.serial('terser preserve vars in nameCache when provided', async (t) => {
 });
 
 test.serial('minify when os.cpus().length === 0', async (t) => {
-  const original = os.cpus;
+  const originalAP = os.availableParallelism;
+  const originalCPUS = os.cpus;
+  os.availableParallelism = null;
   os.cpus = () => [];
   try {
     const bundle = await rollup({
@@ -373,6 +375,7 @@ test.serial('minify when os.cpus().length === 0', async (t) => {
     t.is(output.code, '"use strict";window.a=5,window.a<3&&console.log(4);\n');
     t.falsy(output.map);
   } finally {
-    os.cpus = original;
+    os.availableParallelism = originalAP;
+    os.cpus = originalCPUS;
   }
 });

It passes locally on Node 21.5.0.

[dasa]

@shellscape I can't tell if the tests ran after @easrng last commit. If not, would it be possible to run them again?

[shellscape]

@easrng @dasa there are some conflicts that will need to be resolved before I can force the tests to run again. you can fix the pnpm-lock conflict by running pnpm i in the repo root.