-
Notifications
You must be signed in to change notification settings - Fork 262
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
Fix 647 Support Node 18 as Target #649
Fix 647 Support Node 18 as Target #649
Conversation
- Updated abi_crosswalk.json to include node 18 (current). - Added test coverage for cases of unsupported/unknown targets.
c0006c9
to
bbfd7b2
Compare
Would love to see this merged for the node 18 support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Hey! I noticed CI job is missing for node 18, mind adding that? |
@axrj assuming this was referring me, I wrote this PR (and all others leading to #656) last year using two weeks of @solarwindscloud time. I’m no longer on their paycheck (and I think they moved away from this package, @cheempz can confirm). As said earlier this year in #657, if there is interest from corporate users of the package to fund more active maintenance and/or tackling some of the more prominent open issues, and/or potentially taking it off @mapbox - I should be technically able. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronilan, @ewanharris Would you be willing to collaborate to make a single pull request that supports all current versions of Node.js with successful AppVeyor tests? @ronilan you could bring in the changes from #709 into this pull request and update support for Node.js up to v21 to @ewanharris you could help me to add your By combining our knowledge we should provide a working example of taking the current codebase and patching in updates to Thoughts? |
@cclauss the patch-package process is more for consumers of node-pre-gyp to workaround the lack of updates, it allows patching the I'd be happy to PR an updated |
Just an FYI for anyone interested. I have updated abi_crosswalk.json to node 21 in my fork of node-pre-gyp at @acalcutt/node-pre-gyp , which I forked for the node version of maplibre-native (a fork of mapbox-native). Unfortunately it doesn't seem like anyone is maintaining this here.... since this has been open a while with no interest and now needs further updating. |
I added #711 to update abi_crosswalk.json to node 21 if anyone is interested in updating the support. |
I added a second PR #712 which I think makes the changes requsted by @cclauss in #649 (comment) , which is an alternate to #711 |
Overview
This pull request fixes #647
Change
abi_crosswalk.json
to include node 18 (current) (via script).