Skip to content

Conversation

@etr2460
Copy link
Contributor

@etr2460 etr2460 commented Jan 11, 2026

Description

Fixes generation when 0 CPUs are returned by os.cpus(). This happens in sandbox environments. Note that I did not replace os.cpus() with os.availableParallelism() as that was added in v18 of node and the package.json specifies v16 as the min version. This change matches how node v18 and up implements availableParallelism (source)

Related #10506

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Further comments

As this is a rather trivial change, I didn't add a new unit test for it (would involve mocking os.cpus which didn't seem especially useful). I'm happy to add a test if folks feel it's valuable though!

@changeset-bot
Copy link

changeset-bot bot commented Jan 11, 2026

🦋 Changeset detected

Latest commit: ba4b87c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphql-codegen/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@etr2460 etr2460 force-pushed the fix-zero-length-cpus branch from f7f2b46 to ba4b87c Compare January 11, 2026 02:34
@eddeee888 eddeee888 self-assigned this Jan 11, 2026
Copy link
Collaborator

@eddeee888 eddeee888 left a comment

Choose a reason for hiding this comment

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

Thank you!

@eddeee888 eddeee888 merged commit 8cb7d43 into dotansimha:master Jan 11, 2026
20 checks passed
});

return task.newListr(generateTasks, { concurrent: cpus().length });
return task.newListr(generateTasks, { concurrent: cpus().length || 1 });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point about having unit tests here may not be useful 🤔
Even if we mocked cpus(), I can't think of a good (or meaningful) way to assert that it works.

Agree with merging this first, thank you!

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.

2 participants