Skip to content

fix: correct engines.node floor and document Node.js compatibility - #2638

Open
MsfPablo wants to merge 1 commit into
winstonjs:masterfrom
MsfPablo:fix/2586-honest-node-engines
Open

MsfPablo wants to merge 1 commit into
winstonjs:masterfrom
MsfPablo:fix/2586-honest-node-engines

Conversation

@MsfPablo

Copy link
Copy Markdown

Fixes #2586.

Problem

winston declares engines.node: ">= 12.0.0", but the runtime dependency tree has not actually been Node 12 compatible for some time. The single offender is transitive:

winston@3.19.0
└─┬ @dabh/diagnostics@2.0.8
  └── @so-ric/colorspace@1.1.6

@so-ric/colorspace's published CJS bundle uses:

  • ||= — logical OR assignment, ES2021, Node 15+ (dist/index.cjs.js:1976)
  • Object.hasOwn — Node 16.9+ (dist/index.cjs.js:158, :260)

Because the engines field claims Node 12 support, npm emits no EBADENGINE warning at install time. The first signal a user gets is a SyntaxError: Unexpected token '=' thrown at require('winston'), which is what #2586 reports.

I verified that @so-ric/colorspace is the only package in the production tree that exceeds ES2019, by parsing every .js file in a --omit=dev install with acorn at successively higher ecmaVersion levels:

=== runtime tree files needing > ES2019 ===
ES2021 @so-ric/colorspace node_modules/@so-ric/colorspace/dist/index.cjs.js
(total pkgs scanned) 21

winston's own lib/ is ES2019-clean, so nothing here is self-inflicted.

What this PR does

Deliberately the minimal, non-behavioural option:

  1. engines.node → ">= 16.9.0", the honest floor implied by the dependency tree. Users on older runtimes now get an install-time warning instead of a load-time crash.
  2. A short Node.js compatibility section in the README, per @DABH's suggestion in the issue thread, documenting the constraint and noting the (unsupported) @dabh/diagnostics@2.0.3 pin that several commenters are already using.

What this PR deliberately does not do

It does not pin or downgrade @dabh/diagnostics. That would be a policy call about supporting an EOL runtime, and @DABH already stated in the thread that winston does not support EOL Node versions — so making the metadata honest seemed like the right scope rather than reshaping the dependency tree. Happy to change direction if you'd prefer otherwise.

Two things worth your judgement, since they're policy rather than mechanics:

  • >= 16.9.0 is the technical floor, not necessarily the supported one. CI currently exercises Node 22, 24, and 26. If you'd rather have engines reflect what you actually support and test, say ">= 20" or ">= 22", I'm glad to change it — I picked the technically-derived number because it's the one I could prove, and because it's the least disruptive to existing users.
  • Raising engines is arguably a breaking change for anyone currently installing on Node 12–16, though in practice those installs are already broken at runtime. Retarget the base branch if you'd like this to land in a major.

Verification

All commands run at dd41581 on Node 26.5.0.

npm run lint — 0 errors (10 pre-existing warnings, unchanged from master)
lib/winston/tail-file.js
  63:72  warning  Arrow function has too many statements (30). Maximum allowed is 15  max-statements
  63:72  warning  Arrow function has a complexity of 13. Maximum allowed is 11        complexity

lib/winston/transports/console.js
  52:3  warning  Method 'log' has too many statements (19). Maximum allowed is 15  max-statements
  52:3  warning  Method 'log' has a complexity of 12. Maximum allowed is 11        complexity

lib/winston/transports/file.js
  341:18  warning  'buff' is already declared in the upper scope on line 295 column 9     no-shadow
  402:29  warning  'options' is already declared in the upper scope on line 287 column 9  no-shadow

✖ 10 problems (0 errors, 10 warnings)
npm test — all green
Test Suites: 22 passed, 22 total
Tests:       3 todo, 237 passed, 240 total
Snapshots:   0 total
Time:        42.718 s
npm run test:typescript — clean
> npx --package typescript tsc --project test
(no output)

Courtesy disclosure: I used AI assistance (Claude) while investigating and drafting this change. The dependency analysis, the ES-version scan, and the lint/test/tsc runs above were all executed and checked by me locally. CONTRIBUTING.md doesn't currently state an AI policy — mentioning it in case you'd like one, and happy to follow whatever you prefer.

winston declared `engines.node: >= 12.0.0`, but its runtime dependency
tree has not been Node 12 compatible for some time. `@dabh/diagnostics`
resolves `@so-ric/colorspace`, whose published bundle uses `||=`
(Node 15+) and `Object.hasOwn` (Node 16.9+), so requiring winston on
Node 12 throws a SyntaxError at load time with no install-time warning.

Raise the declared floor to >= 16.9.0 so npm surfaces EBADENGINE instead,
and add a README section spelling out the constraint and the unsupported
workarounds people are already using.

Fixes winstonjs#2586
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: winston3.3.3 resolves to @dabh/diagnostics@^2 → pulls @so-ric/colorspace (ES2021 ||=) and crashes on Node 12

1 participant