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
Update @resvg/resvg-js to v2.0.0 #606
Conversation
Weird, it works for me locally with Node.js 16. The only failure I see is the segmentation fault on Node.js 12 on CI. |
The main branch (resvg v1.x) doesn't pass even in my local tests. So I can't troubleshoot it now, can you check what recent changes to the test case might have caused it to fail? |
@yisibl everything works fine here and on CI with resvg 1.x. This branch here only fails with Node.js 12, and the only change is the resvg update to 2.x. :/ |
@yisibl mind this puppeteer/puppeteer#8009. Maybe you have the same problem |
@Kreeg I've connected the MacBook's power supply and it still doesn't work. Maybe we should look for a more reliable png diff solution. |
My guess is that this issue(swc-project/swc#2276 (comment)) with Rust may be the cause, @Brooooooklyn is helping to investigate further. |
Please be patient, we are still troubleshooting the cause of the crash, I have 100% reproduced the problem on the Mac, for details of the debugging process please see: yisibl#3 |
@yisibl I didn't mean to put more pressure on you, sorry about that! I was just stating my plan :) Thanks for looking into the issue! |
@XhmikosR It doesn't matter |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
@XhmikosR After various debugging, we finally located a possible NAPI bug that was fixed in Node.js 14+, but not cherry-picked to Node.js 12.x. Please go to support this issue at nodejs/node#42376 |
@yisibl thanks for digging into it! I notice the Node.js developers have already added the needed labels, let's hope they pick the patch before Node.js 12 goes EOL. |
a2f372f
to
c614933
Compare
@yisibl I don't suppose it's possible to add |
@XhmikosR Yes, resvg-js v1.x does not intend to add this API. |
Good news, now the last version of Node.js 12.x has been released: 12.22.12-rc.0 🚀🚀🚀, PR #42531. The tests have passed! Download: https://nodejs.org/download/rc/v12.22.12-rc.0/ https://github.com/yisibl/svg-sprite/runs/5781947632?check_suite_focus=true |
You need to change to |
@yisibl I just thought the stable release has already been deployed. I think the better way is just to wait for the release |
Node.js 12.22.12 already released. nodejs/node@a3d2837 |
d9781f6
to
33a8645
Compare
Ok! All tests are passed |
Let's wait until 12.22.12 has landed in the GitHub images. But this makes me wonder if we should bump the package.json version or not. Probably not, since AFAICT things will work fine unless someone has SVGs without dimensions. |
@yisibl do you plan to release v2.0.0 final soon? |
resvg-js v2.0.0 final is released. yisibl/resvg-js@ecc7a56 |
@yisibl that's cool! Thank you! |
It seems Node.js 12 fails with a Segmentation fault
@yisibl do you have any ideas?