Bumped version of which, bumped cmake-ts's version#60
Open
diefbell-grabcad wants to merge 5 commits intoEmbeddedEnterprises:masterfrom
Open
Bumped version of which, bumped cmake-ts's version#60diefbell-grabcad wants to merge 5 commits intoEmbeddedEnterprises:masterfrom
which, bumped cmake-ts's version#60diefbell-grabcad wants to merge 5 commits intoEmbeddedEnterprises:masterfrom
Conversation
Contributor
Author
There was a problem hiding this comment.
pnpm add which@3.0.1 decided it needed to reformat the entire file
Contributor
Author
There was a problem hiding this comment.
Which then made linting fail 🤦
aminya
requested changes
May 28, 2025
Collaborator
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When trying to build
ZeroMQ.js, I got the error[ERROR cmake-ts] Error: not found: ninja. It seems that this is caused by thenothrowoption not being respected herecmake-ts/src/generator.ts
Line 18 in 44d3794
There was some discussion regarding this on the
node-whichrepo, 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 ^_^