chore!: require Node.js 22 or newer#1747
Conversation
Raise the minimum supported Node.js version to 22 and run CI only on currently supported release lines (22, 24, 26). - engines.node: >=18.17 -> >=22 - @types/node range: >=18 -> >=22 - Babel preset-env target: node 18 -> node 22 - CI (nodejs.yml, appveyor.yml): test matrices -> 22.x, 24.x, 26.x; single-version jobs and release.yml -> Node 22 - devcontainer image -> javascript-node:22 With the Node baseline raised, drop the @azure/* overrides block (added only to hold the transitive Azure packages on their Node-18 line) and bump the direct Azure deps to their latest, which now require Node >=20: @azure/core-auth ^1.10.1, @azure/identity ^4.13.1, @azure/keyvault-keys ^4.10.2. Also bump the now-Node-gated devDeps: rimraf ^6.1.3, nyc ^18.0.0, semantic-release ^25.0.5. Verified on Node v22.23.0: npm ci, build, lint (eslint + tsc), and all 394 unit tests pass. BREAKING CHANGE: Node.js 22 is now the minimum supported version. Node.js 18, 20, and 21 are no longer supported. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review:
|
The `codecov` npm package is no longer used: CI uploads coverage via the `codecov/codecov-action@v5` GitHub Action, and no script references the package. It is also deprecated and unmaintained, and pulled a number of vulnerable transitive dependencies (js-yaml, teeny-request, uuid, @tootallnate/once, http-proxy-agent) into the dev dependency tree. Verified on Node v22.23.0: lint (eslint + tsc) and all 394 unit tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
657ce0a to
95673f0
Compare
Code Review:
|
Code Review:
|
The "reads data that is sent across multiple packets, with a chunk
containing multiple packets" test asserted that three packet payloads
arrive merged into a single chunk ([ payload ]). That merging was never
a tedious behavior: it came from Node's Readable.read() (used by the
async iterator) concatenating the whole internal buffer when called
with no size argument.
Node commit fadb214d95 ("stream: readable read one buffer at a time",
nodejs/node#60441) changed howMuchToRead() to return one buffer at a
time, so the async iterator now yields each packet payload as its own
chunk. Node 24 still has the old behavior, Node 26 has the new one,
which is why only the Node 26 CI job failed.
How IncomingMessageStream chunks the payloads is an implementation
detail the test never cared about (the original comment said as much),
so assert on Buffer.concat(receivedData) instead of the exact chunk
boundaries. Verified on both Node v22.23.0 and v26.3.1.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review: chore!: require Node.js 22 or newerOverviewThis PR raises the minimum Node.js version from 18 to 22, narrows the CI matrix to currently-supported release lines (22, 24, 26), bumps direct Azure dependencies that were previously held back by the Node 18 baseline, and removes the now-unused Positive Aspects
Observations & Suggestions1.
|
Update-NodeJsInstallation defaults to the x86 (32-bit) Windows build when no architecture is given. Node.js no longer ships 32-bit Windows builds for the supported release lines (22, 24, 26), so the install step failed. Pass x64 explicitly to install the 64-bit build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review: chore!: require Node.js 22 or newerOverviewThis PR is a well-scoped, infrastructure-only change that raises the Node.js minimum from 18 to 22, updates the CI matrix to Strengths
Observations and Suggestions1. MSAL major version bumps are transitive but non-trivial
2. Dev tooling engines vs. stated minimum
3. Node 26 is "Current", not LTS Including 26.x in the CI matrix is consistent with the old strategy of including one non-LTS release (the old matrix had 21.x). This is fine for early-compat signalling. The PR description already calls out the 27.x omission clearly. Just make sure there is a plan to swap it for a future version once 26 approaches EOL. 4. Updating to 5. CI job naming The fixed-version Azure auth jobs (e.g. No Issues Found With
SummaryThis is a clean, well-prepared infrastructure modernization. The main thing worth confirming before merging is that the Azure integration CI jobs (azure-sql-auth, azure-ad-auth, token-credential-auth, azure-ad-service-principal-auth) pass with the new MSAL 5.x transitive deps — either by waiting for CI to run green or by referencing prior passing runs on the branch. Everything else looks correct and well-reasoned. Verdict: Approve pending Azure CI green. |
Node.js 22+ refuses to run on Windows older than Windows 10 / Windows Server 2016 (exit code 216). AppVeyor's default image, Visual Studio 2015, runs on Windows Server 2012 R2, so the build failed as soon as node was invoked. Pin the Visual Studio 2022 image, which runs on Windows Server 2022. SQL Server 2017 is still pre-installed there, so the existing `mssql2017` service is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review:
|
5569162 to
dfe0df0
Compare
Code Review:
|
|
🎉 This PR is included in version 20.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Raises the minimum supported Node.js version to 22 and narrows CI to the currently supported release lines (22, 24, 26). This unlocks a dependency cleanup that was previously held back by the Node 18 baseline.
Warning
Breaking change. Node.js 22 is now the minimum supported version. Node.js 18, 20, and 21 are no longer supported.
Node version bump
engines.node:>=18.17→>=22@types/noderange:>=18→>=22preset-envtarget:node 18→node 22nodejs.yml,appveyor.yml): test matrices →22.x, 24.x, 26.x; single-version jobs (lint, Azure, pre-release) andrelease.yml→ Node 22javascript-node:22(
27.xis intentionally omitted — Node 27 hasn't been released yet; it can be added once it ships.)Dependency bumps
With the Node baseline raised:
@azure/*overridesblock — it existed only to hold the transitive Azure packages on their Node-18-compatible line (see chore(deps): update dependencies within existing semver ranges #1741). Removing it lets the Azure deps move to their latest, which now declareengines.node >=20.@azure/core-auth^1.10.1,@azure/identity^4.13.1,@azure/keyvault-keys^4.10.2rimraf^6.1.3,nyc^18.0.0,semantic-release^25.0.5codecovnpm package — CI uploads coverage viacodecov/codecov-action@v5, and nothing references the npm package.Deliberately deferred (not Node-driven, would be breaking on their own):
eslint10 — current@typescript-eslint8.x only peers^8 || ^9, so it would force atypescript-eslintv9 majorchai5/6 — ESM-only, requires rewriting test imports@types/lru-cache7 — the code still uses the oldimport LRU from 'lru-cache'APIThe remaining
npm auditadvisories (mocha →serialize-javascript/diff, nyc/babel-plugin-istanbul →js-yaml, and theundicibundled inside the npm CLI used by semantic-release) are all dev-only and will be looked at separately.Verification
Ran on Node v22.23.0:
npm install,npm run build,npm run lint(eslint + tsc), and all 394 unit tests pass.🤖 Generated with Claude Code