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

[api-minor] Update the minimum supported Node.js version to 18 #16293

Merged
merged 4 commits into from
May 7, 2023

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Apr 14, 2023

  • [api-minor] Update the minimum supported Node.js version to 18

    This patch updates the minimum supported environments as follows:

    Note also that Node.js 16 will soon reach EOL, and thus no longer receive any security updates.

  • Remove the compatibility checks in WorkerMessageHandler.createDocumentHandler

    For some time these checks have only targeted Node.js environments, since the features in question exist in all supported browsers (even when a legacy-build is used).

    Now that we've updated the minimum supported Node.js version to 18, a number of polyfills are thus (finally) no longer necessary in that environment. Hence for certain basic functionality, such as e.g. text-extraction, it's now possible to use either a modern- or a legacy-build of the PDF.js library in Node.js environments.

    Please note: For e.g. canvas-rendering in Node.js environments it's still necessary to use a legacy-build, since that functionality requires various polyfills.

  • Remove the IMAGE_DECODERS special-case when polyfilling structuredClone

    Originally we only used the structuredClone polyfill in the LoopbackPort-implementation, and that obviously isn't used anywhere within the various image decoders.
    At this point in time we've started to use structuredClone a little bit more, hence it seems overall simpler to just bundle the polyfill even in the legacy-version of the IMAGE_DECODERS built-target.

@timvandermeij
Copy link
Contributor

I'm in favor of doing this considering that Node 18 is the current LTS version (and for e.g. Firefox we also only support the current ESR version), the upcoming EOL date of Node 16 and the fact that we already updated to Node 18 in the CI as well some time ago in 588447a and didn't run into any issues. Moreover, it helps to remove compatibility code and a dependency, which is always good.

@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Apr 15, 2023

My plan was actually to wait a little bit before submitting this for actual review, since we just increased the minimum supported Node.js version to 16 in the latest release. Unfortunately some recent patches, see e.g. PR #16175, caused various regressions and it'd thus be good to have at least one official release that works in Node.js 16 before support is removed.

the upcoming EOL date of Node 16 and the fact that we already updated to Node 18 in the CI as well some time ago in 588447a and didn't run into any issues.

Part of the problem, w.r.t. the mentioned regressions, is that we only test in Node.js 18 (i.e. the current LTS version) since that allowed the regressions to sneak in. From a PDF.js project perspective it doesn't feel great having to also test in old Node.js versions, and increasing the minimum supported version would indeed considerably simplify maintenance.

This patch updates the minimum supported environments as follows:
 - Node.js 18, which was released on 2022-04-19; see https://en.wikipedia.org/wiki/Node.js#Releases

Note also that Node.js 16 will soon reach EOL, and thus no longer receive any security updates.
…ntHandler`

For some time these checks have only targeted Node.js environments, since the features in question exist in all supported browsers (even when a `legacy`-build is used).

Now that we've updated the minimum supported Node.js version to 18, a number of polyfills are thus (finally) no longer necessary in that environment. Hence for certain *basic* functionality, such as e.g. text-extraction, it's now possible to use either a modern- or a `legacy`-build of the PDF.js library in Node.js environments.

*Please note:* For e.g. canvas-rendering in Node.js environments it's still necessary to use a `legacy`-build, since that functionality requires various polyfills.
…one`

Originally we only used the `structuredClone` polyfill in the `LoopbackPort`-implementation, and that obviously isn't used anywhere within the various image decoders.
At this point in time we've started to use `structuredClone` a little bit more, hence it seems overall simpler to just bundle the polyfill even in the `legacy`-version of the IMAGE_DECODERS built-target.
Now that Node.js version 18 is required, we should be able to use `Array.prototype.at()` everywhere in the code-base.
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/fc8532687eed3fe/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/fc8532687eed3fe/output.txt

Total script time: 1.29 mins

Published

@Snuffleupagus Snuffleupagus marked this pull request as ready for review May 7, 2023 11:51
@Snuffleupagus
Copy link
Collaborator Author

/botio unittest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/b10b1469989df3f/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/5df8ce00bf9347c/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/b10b1469989df3f/output.txt

Total script time: 2.62 mins

  • Unit Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/5df8ce00bf9347c/output.txt

Total script time: 13.21 mins

  • Unit Tests: Passed

@timvandermeij timvandermeij merged commit 7ecb323 into mozilla:master May 7, 2023
@timvandermeij
Copy link
Contributor

Nice work; thanks!

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

Successfully merging this pull request may close these issues.

3 participants