Skip to content

root - fix: allow Node 24+ in engines.node - #488

Merged
jaredwray merged 2 commits into
mainfrom
claude/writr-engines-node-shq7ah
Jul 25, 2026
Merged

root - fix: allow Node 24+ in engines.node#488
jaredwray merged 2 commits into
mainfrom
claude/writr-engines-node-shq7ah

Conversation

@jaredwray

Copy link
Copy Markdown
Owner

Please check if the PR fulfills these requirements

  • Followed the Contributing and Code of Conduct guidelines.
  • Tests for the changes have been added (for bug fixes/features) with 100% code coverage.
    • One-line package.json metadata fix; no source change, so no new tests. The existing suite passes unchanged (186 tests, 100% statements/functions/lines).

What kind of change does this PR introduce?

Bug fix — engines.node currently excludes the Node versions this package is actually tested on and shipped for.

Summary

-"node": "^22.18.0"
+"node": "^22.18.0 || >=24.0.0"

^22.18.0 resolves to >=22.18.0 <23.0.0, so it excludes Node 24 and 26.

Why this looks unintended

The repo already treats 24 as its primary target, and the declaration contradicts that in three places:

Signal Value
.nvmrc 24
.github/workflows/tests.yml node-version: ['22', '24', '26']
.github/workflows/code-coverage.yml, deploy-site.yml node-version: 24
engines.node (before this PR) ^22.18.0 — excludes 24 and 26

So the package declares as unsupported the exact versions its own CI exercises.

It also appears to be a dropped clause rather than a deliberate narrowing:

version engines.node
6.1.2 >=20
6.1.3 ^22.18.0
6.1.4 ^22.18.0

The sibling package docula uses ^22.18.0 || >=24.0.0 — allow 22.18+ and 24+, skip the odd-numbered 23 line. This PR restores that form, which matches the intent the rest of the repo already expresses.

Consumer impact

Installing any package that depends on writr under Node 24+ with engine-strict=true currently aborts with ERR_PNPM_UNSUPPORTED_ENGINE before any code runs. Default installs are unaffected, since pnpm and npm only enforce engines when strict mode is on — which is why this went unnoticed.

Surfaced downstream in jaredwray/ecto#389, where ecto depends on writr and supports Node 22/24/26. Because ecto declares writr: ^6.1.2, consumers already resolve to 6.1.4, so they are exposed today.

Verification

  • pnpm install --frozen-lockfile clean
  • pnpm build passes
  • pnpm test:ci passes — 186 tests, 100% statements / 100% functions / 100% lines / 98.76% branches (unchanged from main)

No lockfile change — engines does not affect resolution.


Generated by Claude Code

- engines.node "^22.18.0" -> "^22.18.0 || >=24.0.0"

"^22.18.0" resolves to >=22.18.0 <23.0.0, so it excludes Node 24 and 26 even
though .nvmrc pins 24 and .github/workflows/tests.yml runs the suite on 22, 24,
and 26. The package was declaring unsupported the exact versions it tests on.

The constraint was introduced in 6.1.3 (6.1.2 declared ">=20") and looks like a
dropped clause rather than an intentional narrowing: the sibling package docula
uses "^22.18.0 || >=24.0.0", which allows 22.18+ and 24+ while skipping the
odd-numbered 23 line. This restores that form.

Consumer impact: installing a package that depends on writr under Node 24+ with
engine-strict=true currently fails with ERR_PNPM_UNSUPPORTED_ENGINE before any
code runs. Surfaced downstream in jaredwray/ecto#389.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018i8MS4p9jRHqTWqGW6JXQE

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the supported Node.js engine versions in package.json to include Node.js version 24 or higher in addition to version 22.18.0. There are no review comments, and I have no feedback to provide.

@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (cdb382f) to head (f70fa36).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #488   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          518       518           
  Branches       144       144           
=========================================
  Hits           518       518           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The harness-parity "Browser smoke (Chromium)" step ran the browser installer
from the repo root, where playwright-core is not a dependency. npx therefore
fetched the newest release from the registry instead of using the copy the
smoke test actually loads.

browser-smoke.mjs resolves playwright-core from writr-rs/crates/writr-node
(createRequire against that package.json), which pins ^1.61.1 -> 1.61.1 ->
chromium-headless-shell revision 1228. playwright-core 1.62.0 published
2026-07-24 and installs revision 1234, so the installer downloaded 1234 while
the launcher looked for 1228:

  browserType.launch: Executable doesn't exist at
  .../ms-playwright/chromium_headless_shell-1228/...

Running the installer from crates/writr-node with pnpm exec uses the pinned
binary, so installer and launcher always agree and the step cannot drift again
when a new playwright-core ships.

This break is independent of the engines.node change in this branch: it is
latent on main, which last ran writr-rs on 2026-07-16, eight days before
1.62.0 was published.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018i8MS4p9jRHqTWqGW6JXQE

Copy link
Copy Markdown
Owner Author

harness-parity failed on cf8efa4. It is not caused by the engines.node change — it is a latent CI break that landed yesterday and affects every writr PR opened since. I pushed a fix in f70fa36 because this PR cannot go green without it; happy to split it into its own PR if you'd rather keep this one to the one-line metadata change.

What failed

browserType.launch: Executable doesn't exist at
  .../ms-playwright/chromium_headless_shell-1228/chrome-headless-shell-linux64/chrome-headless-shell

The job downloaded browser build 1234 and then tried to launch 1228.

Why

Two different playwright-core versions were in play in the same step:

resolves from version chromium build
installer (npx playwright-core install) repo root — not a dependency there, so npx fetches latest from the registry 1.62.0 1234
launcher (browser-smoke.mjs) writr-rs/crates/writr-node via createRequire, pinned ^1.61.1 1.61.1 1228

browser-smoke.mjs:23-24 binds the launcher to the binding package:

const require = createRequire(join(nodeDir, "package.json"));
const { chromium } = require("playwright-core");

but the workflow step has no working-directory, so the installer ran at the root where playwright-core is not declared. npx fell back to fetching the newest release.

Why it started now

playwright-core@1.62.0 was published 2026-07-24 21:56 UTC. The last writr-rs run on main was 2026-07-16 (run 29534609104), where harness-parity — Browser smoke included — passed. Until 1.62.0 shipped, the registry's latest still mapped to build 1228, so root-level npx happened to agree with the pin. It no longer does. main would fail identically if re-run today.

The fix

-npx playwright-core install chromium --with-deps || npx playwright install chromium --with-deps
+(cd writr-rs/crates/writr-node && pnpm exec playwright-core install chromium --with-deps)

pnpm exec uses crates/writr-node/node_modules/.bin/playwright-core, the same copy the smoke test loads, so installer and launcher agree by construction and cannot drift apart when a future playwright-core ships. The || npx playwright install fallback is dropped — it existed to cover the missing-binary case, which is exactly what pulled in the wrong version.

Verified locally: crates/writr-node resolves playwright-core@1.61.1, whose browsers.json lists chromium-headless-shell revision 1228, matching what the launcher expects.


Generated by Claude Code

@jaredwray
jaredwray merged commit afd4137 into main Jul 25, 2026
17 checks passed
@jaredwray
jaredwray deleted the claude/writr-engines-node-shq7ah branch July 25, 2026 19:00
@jaredwray jaredwray mentioned this pull request Jul 25, 2026
6 tasks
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.

1 participant