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

Fix node-canvas source compile for node >= v21 #4122

Merged
merged 1 commit into from
May 17, 2024

Conversation

springmeyer
Copy link
Contributor

The maplibre-gl-js install currently fails on arm64 macs when running Node >= 21.

The error is:

../../nan/nan.h:2546:8: error: no matching member function for call to 'SetAccessor'
tpl->SetAccessor(

The context is that maplibre-gl-js depends on node-canvas@2.x which has a dependency on the nan package and nan 2.17 had a bug that surfaced with node v21: nodejs/nan#966.

Additionally node-canvas does not currently provide pre-compiled binaries for Arm64, so a source compile is needed.

So, the solution here is to upgrade the nan version to the latest release which fixes support with recent node.

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

The maplibre-gl-js install currently fails on arm64 macs when running
Node >= 21.

The error is:

```
../../nan/nan.h:2546:8: error: no matching member function for call to 'SetAccessor'
tpl->SetAccessor(
```

The context is that `maplibre-gl-js` depends on `node-canvas@2.x` which has a
dependency on the `nan` package and `nan` 2.17 had a bug that surfaced with
node v21: nodejs/nan#966.

Additionally `node-canvas` does not currently provide pre-compiled
binaries for Arm64, so a source compile is needed.

So, the solution here is to upgrade the `nan` version to the latest
release which fixes support with recent node.
@codecov-commenter
Copy link

codecov-commenter commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.63%. Comparing base (0b4688b) to head (da52528).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4122      +/-   ##
==========================================
+ Coverage   86.60%   86.63%   +0.02%     
==========================================
  Files         242      242              
  Lines       33041    33041              
  Branches     2023     2019       -4     
==========================================
+ Hits        28616    28624       +8     
+ Misses       3448     3435      -13     
- Partials      977      982       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HarelM
Copy link
Collaborator

HarelM commented May 16, 2024

Wouldn't a newer version of node-canvas will solve this in the future?
Node 20 is what the CI and everything is supporting and we usually only use LTS to be on the safe side...
What's the motivation behind using node 21 and working on this library?

@springmeyer
Copy link
Contributor Author

Thanks for your thoughtful questions @HarelM

Wouldn't a newer version of node-canvas will solve this in the future?

Yes. Upgrading to a newer node-canvas would also solve this, if/when a newer version is released (because the package-lock.json would get updated in the same way this PR updates it). Currently maplibre-gl-js uses the latest release from last year: v2.11.2 and I don't see a newer release available. I did notice that the main branch of node-canvas declares a 3.x series. So in anticipation that 3.x might bring breaking changes and pose difficulty upgrading, it seemed like working around this problem in maplibre-gl-js was reasonable. Or we could advocate for a new node-canvas v.2.x release.

Node 20 is what the CI and everything is supporting and we usually only use LTS to be on the safe side...

Good to know.

What's the motivation behind using node 21 and working on this library?

Node v22 will enter LTS in 5 months (https://nodejs.org/en/blog/announcements/v22-release-announce). I happened to be running it when I built latest maplibre today, so I was motivated to help others in the future avoid this potential install problem.

@HarelM HarelM enabled auto-merge (squash) May 17, 2024 21:02
@HarelM
Copy link
Collaborator

HarelM commented May 17, 2024

Fair enough, thanks!

@HarelM HarelM merged commit 753204c into maplibre:main May 17, 2024
15 checks passed
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.

3 participants