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

Upgrade canvas to version 3.0.0 #18049

Closed
timvandermeij opened this issue May 6, 2024 · 9 comments · Fixed by #18922
Closed

Upgrade canvas to version 3.0.0 #18049

timvandermeij opened this issue May 6, 2024 · 9 comments · Fixed by #18922

Comments

@timvandermeij
Copy link
Contributor

In PR #18009 the GitHub Actions CI workflow has temporary been updated to not use the latest Node.js version (22 as of writing) but Node.js 21 instead because the canvas dependency doesn't provide pre-built binaries for Node 22.js yet. We don't want to build it ourselves because that'd require additional complexity for local and workflow installations, and sadly it looks like this may take some time to resolve upstream given that Automattic/node-canvas#2377 and the corresponding release tracking issue Automattic/node-canvas#2232 didn't have recent updates. To avoid having failing builds in the meantime, we chose to pin Node.js to version 21 for the time being.

If Automattic/node-canvas#2377 is resolved upstream and a new release is published with pre-built binaries, we should update to that new version and revert e561a4a so we test against the latest version again.

@Snuffleupagus
Copy link
Collaborator

Given that Node.js 21 is no longer maintained, according to https://github.com/nodejs/Release/blob/6209d04302e62156b964a605f619283582334c95/schedule.json#L117-L121, should we stop testing in that environment?

@timvandermeij
Copy link
Contributor Author

timvandermeij commented Jun 20, 2024

On the one hand I think we should, but on the other hand it's the highest version we have at the moment because switching back to latest is still blocked on node-canvas releasing a new stable version with the fix. Ideally we'd wait for that, but given the slow movement of that issue I think we can consider using the unstable release mentioned in Automattic/node-canvas#2377 (comment) for the time being. Now that there is an official unstable release I'm more comfortable with that, assuming the required prebuilds are in fact available, because the previously mentioned approach in that thread of using the master branch without a release really didn't seem like a viable solution to me.

What do you think about this?

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Jun 20, 2024

What do you think about this?

I actually tried updating that earlier today, but unfortunately it didn't seem to work on GitHub actions; here's the patch: master...Snuffleupagus:pdf.js:node-canvas-3

@timvandermeij timvandermeij changed the title Remove the Node.js 21 pin in the GitHub Actions CI workflow Upgrade canvas to version 3.0.0 Jul 21, 2024
@timvandermeij
Copy link
Contributor Author

Testing with Node 21 has been removed in #18505, so we can only more forward with testing with Node 22 or newer once there is a resolution for the canvas dependency.

@GitMurf
Copy link

GitMurf commented Aug 12, 2024

@timvandermeij would you mind briefly explaining what the implications are here short term? Should we be using node-canvas@next? Is there anything special we should be doing / configuring or keeping an eye out for? Thanks!

@timvandermeij
Copy link
Contributor Author

For now we keep using node-canvas 2.x together with Node < 22 until node-canvas 3.x is released.

@kotasudhakar

This comment was marked as outdated.

@Snuffleupagus
Copy link
Collaborator

They have now released 3.0 beta

There's not been any recent releases, so nothing has changed since #18049 (comment) was made. (And please refrain from needlessly tagging users, since that creates notification spam.)
The latest pre-release, which is v3.0.0-rc2 as of this writing, unfortunately suffers from a pretty trivial bug that prevents usage; see Automattic/node-canvas#2415

@Snuffleupagus
Copy link
Collaborator

The latest pre-release, which is v3.0.0-rc2 as of this writing, unfortunately suffers from a pretty trivial bug that prevents usage; see Automattic/node-canvas#2415

This has been fixed, and installing node-canvas seems to work now; see master...Snuffleupagus:pdf.js:node-canvas-3

However, running gulp unittestcli either locally or on GitHub Actions fails with the following output:

[11:34:57] Starting 'runUnitTestCli'...
[11:34:57] 'runUnitTestCli' errored after 131 ms
[11:34:57] Error: Unit tests failed.
    at ChildProcess.<anonymous> (file:///home/runner/work/pdf.js/pdf.js/gulpfile.mjs:1896:14)
    at ChildProcess.emit (node:events:519:28)
    at ChildProcess.emit (node:domain:551:15)
    at maybeClose (node:internal/child_process:1105:16)
    at ChildProcess._handle.onexit (node:internal/child_process:305:5)
    at Process.callbackTrampoline (node:internal/async_hooks:130:17)
[11:34:57] 'unittestcli' errored after 8.13 s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants