Repository navigation
chore: raise minimum supported Node to 22 and test 24 and 26 - #383
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #376, which had to drop the Node 20 job because its test runner flakes on macOS. That fix left a gap:
enginesand the docs still claimed Node 20 support while CI no longer tested it.The floor moves to 22
v22.x-staging(#65807), so the one remaining known flake in the matrix should clear on its own.Why test 26 as well as 22 and 24
nvm install nodegives today26 becomes the recommended LTS in under a month. Testing it now is how breakage gets found before that happens rather than after. It also already carries the deserialization fix (landed in 26.7.0), so it adds no flake risk.
The floor is defined once
src/api/doctor.jsread the minimum from two literals on adjacent lines — the same driftsecurity.jshad before #374. It now derives it frompackage.json:pkgwas already loaded frompackage.jsonat that point, so this adds no file read and no new constant. npm's install-time enforcement androlecraft doctornow agree by construction rather than by convention.Regression test
doctorhad no coverage for the floor at all — only a test asserting it reports the current version. Added one that stubsprocess.versions.nodeto one below the floor and to the floor itself:>= 22 requiredVerified it fails when the comparison is broken (
major < MIN_NODE_MAJOR→major < 0).The test reads the floor from
package.jsonrather than hardcoding 22, deliberately: it pins the agreement between npm and doctor, which is the thing that can silently break. Pinning the literal would just create a second place to update when the floor moves again.Updated
.github/workflows/test.yml— matrix[22, 24, 26], with the Node 20 removal reason and each line's role recorded next to itpackage.json—engines.node: ">=22".nvmrc—v20→v22README.md,CONTRIBUTING.md,docs/install.md,docs/guides/getting-started.md— requirementsdocs/guides/ci.md— the copy-paste CI example pinnednode-version: 20, which would have handed users a version this repo no longer supportsdocs/commands/doctor.md— documents the check, which now enforces 22MANIFEST-MATRIX.md+ generated tokens —test_count1168 → 11691269/1269 pass, lint clean.
Follow-up worth noting
When 26 goes LTS on 2026-10-28 and 24 enters maintenance on 2026-10-20, the reference version for the audit/coverage/lint jobs (
matrix.node-version == 24) should probably move to 26. Left alone here to keep this PR to the floor change — worth doing once 26 is actually LTS, not before.