Skip to content

Fix: Raise minimum Node version to 22 (fixes #3821) - #3822

Merged
oliverfoster merged 1 commit into
masterfrom
issue/3821
Aug 4, 2026
Merged

Fix: Raise minimum Node version to 22 (fixes #3821)#3822
oliverfoster merged 1 commit into
masterfrom
issue/3821

Conversation

@swashbuck

@swashbuck swashbuck commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #3821

Fix

  • Raises engines.node to >=22, so the declared minimum matches a Node version where globalThis.crypto exists and grunt build actually completes. Since v5.56.0 the declared minimum of >=18 named a version the build cannot run on at all — @rollup/plugin-terser@1.0.0 pulls serialize-javascript@7, which calls the global crypto.getRandomValues() at module scope, and grunt/tasks/javascript.js requires the plugin at task-load time. Both of those packages already declare >=20.0.0 themselves.
  • Updates the Requirements line in the README, which still said Node >=16 and was already two floors out of date.

Testing

  1. On Node 22 or 24, npm ci && npx -y -p adapt-cli adapt install && npx grunt build completes and produces adapt.min.js.
  2. On Node 18 or 20, npm install now reports the engine mismatch for the framework itself (npm warn EBADENGINE) rather than only for its dependencies, so the incompatibility is visible at install time instead of surfacing as Warning: crypto is not defined at build time.

Notes for reviewers

  • A revert is not an option here. Chore: Bump serialize-javascript and @rollup/plugin-terser #3791 was a security bump: serialize-javascript 6.0.2 carries GHSA-5c6j-r48x-rmvq (high, remote code execution, patched in 7.0.3) and GHSA-qj8w-gfj5-8c6v (moderate, denial of service, patched in 7.0.5). Pinning back reintroduces both and Dependabot would reopen the PR. There is also no in-repo shim: @rollup/plugin-terser@1.0.0 runs terser in a worker thread pool and each worker requires serialize-javascript again in its own global context, so setting globalThis.crypto in Gruntfile.js does not reach the workers — only a process-level NODE_OPTIONS=--require preload propagates.
  • Why 22 and not 20. Every Node version this bug breaks is already end of life — v16 since August 2023, v18 since March 2025, v20 since March 2026. Setting the floor to >=20 would pin to a line that no longer receives security patches. 22 and 24 are the current LTS lines.
  • Release type is a maintainer call. This is submitted as Fix on the grounds that nothing works today that stops working — builds on Node 18 and 20 already fail. If you would rather the support-floor change drive a major version, the squash-merge subject needs to be Breaking instead.
  • Worth stating the new floor in the release notes and the authoring tool compatibility matrix. Legacy adapt_authoring 0.11.5 declares engines: { node: "16 || 18" }, so it sits entirely below this floor; its declared range is advisory rather than enforced, and a fresh install has been verified to run and publish successfully on Node 22.
  • Two related follow-ups deliberately left out of this PR, both mentioned in Fix "crypto is not defined" build failure on Node 16 and 18 introduced in v5.56.0 #3821 and neither needed to fix it. Happy to raise either separately if wanted. The CI workflows still pin node-version: 'lts/*', so the declared floor is never the version actually exercised — that is what let a minimum of 18 go untested for nearly four months. And Framework.getSchemas() does not forward warn, so plugin warnings fall back to console.warn on stderr, which is why an unrelated warning was mistaken for the failure in this bug's reports.

Posted via collaboration with Claude Code

@simondate

simondate commented Aug 4, 2026

Copy link
Copy Markdown
Member

Is there any potential side effects introduced by going up to Node 22? I thought the AAT legacy branch was to be kept the same? Couldn't we just keep that product on a stable Node 16/18 version of the FW?

EDIT: Seems like the AAT works fine with the newer node version (from my brief testing of features and builds).

Could be some other side effect with in the FW though?

@swashbuck

Copy link
Copy Markdown
Contributor Author

Is there any potential side effects introduced by going up to Node 22?

I thought the AAT legacy branch was to be kept the same? Couldn't we just keep that product on a stable Node 16/18 version of the FW?

@oliverfoster @taylortom Any thoughts here?

@simondate
simondate self-requested a review August 4, 2026 16:51
@oliverfoster
oliverfoster merged commit afac25e into master Aug 4, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from Needs Reviewing to Recently Released in adapt_framework: The TODO Board Aug 4, 2026
@oliverfoster
oliverfoster deleted the issue/3821 branch August 4, 2026 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Recently Released

Development

Successfully merging this pull request may close these issues.

Fix "crypto is not defined" build failure on Node 16 and 18 introduced in v5.56.0

3 participants