Pin GitHub actions to Node.js 21

[nicolo-ribaudo]

Node.js 22 was just released, and it seems like it's not compatible with the canvas package. This commit pins the version on GitHub actions to Node.js 21 as a temporary workaround.

This commit should be reverted once Automattic/node-canvas#2377 is fixed.

I noticed CI failing in #17923 and #18001 due to the new Node.js release, and while I don't know enough about node-gyp to fix it this should at least allow running tests until when it's fixed upstream. You might need to to the same in the Bot that runs the tests.

[Snuffleupagus]

This commit should be reverted once Automattic/node-canvas#2377 is fixed.

Maybe we can wait and see how long that issue takes to be fixed and a new version released, before changing anything here?
(Since we also test in Node.js 18 and LTS, we're not losing that much by having this one test fail temporarily as far as I can tell.)

[timvandermeij]

I think getting this resolved upstream might end up taking a while, seeing that the last release is from April last year and it looks like the fix will be part of a new major version for which the tracking issue (Automattic/node-canvas#2232) is also already open since April last year with no activity in this year so far.

I would therefore propose that we merge this so that all CI builds are green again, and that at the same we open a tracking issue here to revert this once the upstream fix is released. This is mainly because I think it's good to strive to keep the CI green at all times to avoid "getting used" to red checks (possibly resulting in oversights).

Finally, another benefit I see is that if we test against Node 21 we're testing against a newer release than we currently do because, given that the dependencies won't install in Node 22, we're now effectively only really testing against Node 18 and 20.

Would this be a reasonable approach for the time being, or is it not a good idea?

[Snuffleupagus]

I would therefore propose that we merge this so that all CI builds are green again, and that at the same we open a tracking issue here to revert this once the upstream fix is released.

Sure, as long as we track this properly (such that we don't accidentally get "stuck" on Node.js 21).

[nicolo-ribaudo]

Looking at the issues in the node-canvas repo it looks like pdf.js could depend directly on https://github.com/Automattic/node-canvas/releases/tag/v3.0.0-rc1b (from GitHub rather than from npm), which fixes Node.js 22 compatibility but it doesn't provide prebuilt binaries.

If there is interest I can experiment with it.

[Snuffleupagus]

Looking at the issues in the node-canvas repo it looks like pdf.js could depend directly on https://github.com/Automattic/node-canvas/releases/tag/v3.0.0-rc1b (from GitHub rather than from npm), which fixes Node.js 22 compatibility but it doesn't provide prebuilt binaries.

We usually try to avoid having "unstable" dependencies unless absolutely necessary, to reduce maintenance burden, and the lack of binaries would likely add another level of (in my opinion undesirable) complexity unfortunately.

[timvandermeij]

We usually try to avoid having "unstable" dependencies unless absolutely necessary, to reduce maintenance burden, and the lack of binaries would likely add another level of (in my opinion undesirable) complexity unfortunately

I agree; let's apply this patch for now since it seems like the simplest solution to make the builds green again. I have opened #18049 to track reverting this as soon as a new canvas release with pre-built binaries is available.

Thank you for providing the patch @nicolo-ribaudo!