Skip to content

Bumped version of which, bumped cmake-ts's version #60

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

diefbell-grabcad
Copy link
Contributor

@diefbell-grabcad diefbell-grabcad commented May 28, 2025

When trying to build ZeroMQ.js, I got the error [ERROR cmake-ts] Error: not found: ninja. It seems that this is caused by the nothrow option not being respected here

const ninja = await which("ninja", { nothrow: true })
.

There was some discussion regarding this on the node-which repo, seems the issue is fixed in v3+: npm/node-which#80. There are no typings for v4+ so just went for v3.

I've also added the minimum Node engine version for building cmake-ts (not for using cmake-ts) as 18, because that's what it is. Otherwise I get the error: SyntaxError: The requested module 'node:fs/promises' does not provide an export named 'constants'. The Node version used in the GA workflow should probably match the minium expected Node version required for build.

Please also add into ZeroMQ after merge ^_^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pnpm add [email protected] decided it needed to reformat the entire file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which then made linting fail 🤦

Copy link
Collaborator

@aminya aminya left a comment

Choose a reason for hiding this comment

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

Thanks for the change. However, since cmake-ts is a build tool that can be used on older Nodejs to build binaries, we want to keep the code compatible with old versions. Does this update break compatibility?

We can create our own which nothrow, if that's the only reason for the bump. Alternatively, we can add polyfils to maintain compatibility.

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