Repository navigation
fix: align supported Node engine range - #222
Conversation
|
@poketopa is attempting to deploy a commit to the Toss Team on Vercel. A member of the Team first needs to authorize it. |
marshallku
left a comment
There was a problem hiding this comment.
LGTM. The previous range was not accurate anyway: index.js already calls require('node:fs'), which needs Node 14.18/16+, and CI only tests Node 20/24/26. So >=20 now matches what is actually supported and tested.
Checked locally: merges cleanly into main, and the three touched files stay consistent with each other.
Severity: P1 must fix before merge · P2 should fix before merge · P3 worth fixing · P4 suggestion.
| }, | ||
| "engines": { | ||
| "node": ">= 10.20.0 < 11 || >= 12.17.0 < 13 || >= 14.0.0" | ||
| "node": ">=20" |
There was a problem hiding this comment.
P4 · release — This narrows the declared support range, so users on Node 18 will start getting an EBADENGINE warning (or an install failure with engine-strict). fix: is fine since the old range was never really supported, but it would be worth a line in the release notes, e.g. "Node.js 20 or later is now the declared minimum, matching what CI tests."
Summary
package.json, matching the oldest runtime covered by binding CIRationale
PR #209 removed Node.js 18 from the test matrix and the v0.7.0 release notes record that support change. The binding matrix now verifies Node.js 20, 24, and 26 across macOS, Windows, and Linux, while the package metadata and installation guides still advertised Node.js 10-era support.
Validation
yarn install --immutablejust formatjust lintjust typecheckjust testgit diff --check