[api-minor] Replace the canvas package with @napi-rs/canvas
#19015
Conversation
Snuffleupagus
force-pushed
the
@napi-rs/canvas
branch
3 times, most recently
from
26fbe07 to
aaea218
Compare
|
Before moving forward with this, the following points should probably be agreed upon:
|
Snuffleupagus
force-pushed
the
@napi-rs/canvas
branch
from
930795c to
c85ba9a
Compare
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/daf83370e7d6cfc/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/7c80a5eca86a72b/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/7c80a5eca86a72b/output.txt Total script time: 30.85 mins
Image differences available at: http://54.241.84.105:8877/7c80a5eca86a72b/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/daf83370e7d6cfc/output.txt Total script time: 51.54 mins
|
|
I tested the patch on mac with the example and it works fine. |
The `@napi-rs/canvas` package has fewer dependencies, which should *hopefully* make installing and using it easier for `pdfjs-dist` end-users. (Over the years we've seen, repeatedly, that `canvas` can be difficult to install successfully.) Furthermore, this package includes more functionality (such as `Path2D`) which reduces the overall number of dependencies in the PDF.js project. One point to note is that `@napi-rs/canvas` is a fair bit newer than `canvas`, and has a lot fewer users, however looking at the commit history it does seem to be actively maintained. Note that I've successfully tested the [Node.js examples](https://github.com/mozilla/pdf.js/tree/master/examples/node), in particular the `pdf2png` one, with this patch applied and things appear to work fine. Please see: - https://www.npmjs.com/package/@napi-rs/canvas - https://github.com/Brooooooklyn/canvas
Given that `ImageData` has been supported for many years in all browsers, see [MDN](https://developer.mozilla.org/en-US/docs/Web/API/ImageData#browser_compatibility), we have a `typeof` check that's only necessary in Node.js environments. Since the `@napi-rs/canvas` package provides that functionality, we can thus add an `ImageData` polyfill which allows us to ever so slightly simplify the code.
Snuffleupagus
force-pushed
the
@napi-rs/canvas
branch
from
c85ba9a to
9b62f2e
Compare
I think we should, primarily given the number of installation issues we've seen and the lack of active development, both visible at https://github.com/Automattic/node-canvas too.
I think so. It's under active development, it has a good amount of support/stars from the community, it's written in a memory-safe language (Rust) and it has no other dependencies which should make it easier to install for end users. Doing so also doesn't require many changes as shown in this patch, and it reduces the number of dependencies ever so slightly. The only drawbacks I can imagine are if features that we rely on are missing, and that it's a relatively new project which means that the future could be a bit more uncertain compared to more "settled" packages (but we can always switch later on, so I don't see that as a blocker here). |
The
@napi-rs/canvaspackage has fewer dependencies, which should hopefully make installing and using it easier forpdfjs-distend-users. (Over the years we've seen, repeatedly, thatcanvascan be difficult to install successfully.)Furthermore, this package includes more functionality (such as
Path2D) which reduces the overall number of dependencies in the PDF.js project.One point to note is that
@napi-rs/canvasis a fair bit newer thancanvas, and has a lot fewer users, however looking at the commit history it does seem to be actively maintained.Note that I've successfully tested the Node.js examples, in particular the
pdf2pngone, with this patch applied and things appear to work fine.Please see: