Skip to content
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

getBestNapiBuildVersion() fails on Node 23.6.0 #6

Closed
baileympearson opened this issue Jan 13, 2025 · 1 comment · Fixed by #7
Closed

getBestNapiBuildVersion() fails on Node 23.6.0 #6

baileympearson opened this issue Jan 13, 2025 · 1 comment · Fixed by #7
Assignees
Labels
bug Something isn't working

Comments

@baileympearson
Copy link

Node 23.6.0 increased the napi version to 10 - the first napi version to have double digits. This causes getBestNapiBuildVersion (and perhaps other functions too) to fail to produce the correct version:

$ cat package.json | jq '.binary' 
{
  "napi_versions": [
    4
  ]
}
$ node --version
v23.5.0
$ node -p "require('napi-build-utils').getBestNapiBuildVersion()"
4
$ nvm use 23.6.0
$ node --version                                                 
v23.6.0
$ node -p "require('napi-build-utils').getBestNapiBuildVersion()"
undefined

The root cause is string comparison instead of numeric comparison - until we reached 10, when napiBuildVersion <= ourNapiVersion compared strings, the result was the same as though they were numbers. Now its using lexographic comparison, and napiBuildVersion <= ourNapiVersion -> '4' <= '10' -> 'false'.

@jschlight jschlight self-assigned this Jan 13, 2025
@jschlight jschlight added the bug Something isn't working label Jan 13, 2025
jschlight added a commit that referenced this issue Jan 16, 2025
jschlight added a commit that referenced this issue Jan 16, 2025
jschlight added a commit that referenced this issue Jan 16, 2025
jschlight added a commit that referenced this issue Jan 16, 2025
jschlight added a commit that referenced this issue Jan 16, 2025
jschlight added a commit that referenced this issue Jan 17, 2025
jschlight added a commit that referenced this issue Jan 17, 2025
@jschlight jschlight linked a pull request Jan 17, 2025 that will close this issue
@jschlight
Copy link
Member

This issue should now be addressed in 2.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants