Could someone take a look at updating node-canvas N-API implementation that Jason did back in 2017?
GitMurf opened this issue · 6 comments
@jasongin did a wonderful job at porting node-canvas from NaN to N-API via node-addon-api back in 2017 but there has not been an update since and unfortunately node-canvas has some key things they have changed since then that makes the old fork unusable with any libraries that depend on node-canvas today.
See details here: Automattic/node-canvas#923 (comment)
Here is that fork of his from 2017: https://github.com/jasongin/node-canvas/tree/napi
I have spent weeks trying to figure this out but it is over my head. I started from the NaN to N-Api conversion tool but I can't for the life of me figure out how to do the rest of the cleanup / conversion to make the newest version of node-canvas work (I have zero C/C++ experience so it is a bit foreign to me).
Is anyone willing to take a look to see if you can port the current version of node-canvas to N-Api? Or even if you just port over a couple of the cc/h Class files then maybe that could be enough for me to use as a template to get the rest of it converted.
Thanks in advance!
I also believe updating this library would be a great case study for updating the conversion.js script as there are a lot of repeated patterns in this NaN to Napi library conversion (after running the conversion.js script) that I have had to apply as I have tried to figure the conversion out on my own.
In other words, I think the conversion.js script could be greatly enhanced by taking some of the learnings of the extra conversions that must be done with this node-canvas library after running the initial conversion.js script.
Let me know if you have any questions... thanks!
@GitMurf sorry if I'm answering only now, but you're tright it could be interesting take a look at node-canvas. Now I'm in vacation and I will try to work on this starting from 3rd January 2023.
Hi @NickNaso … hope 2023 has been treating you well! Checking in to see if you’ve considered taking a look at node canvas? :) thanks in advance!
Looks like @chearon finished the port of node-canvas to N-API and just opened a PR for it. Would be wonderful if @NickNaso or someone from the node addon team could take a look and provide any comments or feedback! Thanks a bunch in advance! Here is the PR: Automattic/node-canvas#2235
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.
This has been completed! Nice work by everyone involved! 🎉 Especially @chearon with the help of @zbjornson and @gabrielschulhof!
See merged PR here: Automattic/node-canvas#2235