Skip to content

Commit 68ed33b

Browse files
aryanorastarclaude
andauthored
fix(desktop-windows): name the supported Node version instead of failing silently (#10665)
`pnpm test` in desktop/windows fails 29 test files and 129 tests on a current Node, with the same code and the same lockfile that is fully green on Node 22. Nothing in the package says which Node is required: there is no `engines` field and no `.nvmrc` anywhere in the repo, while all five CI workflows pin `node-version: 22`. The breakage is therefore invisible in CI and lands only on a contributor's machine. The cause is not the tests. Node >= 24 ships its own experimental `localStorage` global, `undefined` unless `--localstorage-file` is passed, and it takes precedence over the one jsdom installs. Every `// @vitest-environment jsdom` suite that touches localStorage then dies on `Cannot read properties of undefined (reading 'clear')`, which reads like a broken test rather than a wrong runtime. Declares the range and enforces it where the symptom appears: - `engines.node` = `>=22.19.0 <23`. The floor is real, not decorative: `@earendil-works/pi-ai` declares `engines.node >= 22.19.0`, so an older 22.x installs and is still unsupported. - A `pretest` guard that names the required range, the running version, and the reason, so the first failure is one legible line instead of 129 assertions. `engine-strict=true` in .npmrc was tried and rejected: it makes pnpm enforce the engines field of every transitive dependency, a far wider blast radius than this problem, and it rejected Node 22.16.0 over a dependency's floor while giving no hint that the test suite itself was fine. Verification (same tree, same lockfile, only the runtime changed): Node 26.3.1 pnpm test -> 29 files / 129 tests FAILED (localStorage undefined) Node 22.23.1 pnpm test -> 523 files / 5037 tests passed, 6 skipped guard on v26.3.1 -> exit 1, explains the localStorage shadowing guard on v22.16.0 -> exit 1, explains the >=22.19.0 dependency floor guard on v22.23.1 -> exit 0 (this is what CI's `node-version: 22` resolves to, so `pnpm test` in CI keeps passing) Failure-Class: none Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 78c8faa commit 68ed33b

2 files changed

Lines changed: 43 additions & 0 deletions

File tree

desktop/windows/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
"version": "1.0.23",
44
"description": "An Electron application with React and TypeScript",
55
"main": "./out/main/index.js",
6+
"engines": {
7+
"node": ">=22.19.0 <23"
8+
},
69
"author": "Based Hardware",
710
"homepage": "https://github.com/BasedHardware/omi",
811
"scripts": {
@@ -34,6 +37,7 @@
3437
"build:win": "npm run build && electron-builder --win --x64 --config electron-builder.config.mjs --publish never",
3538
"build:mac": "electron-vite build && node scripts/bundle-pimono-extension.mjs && electron-builder --mac --config electron-builder.config.mjs --publish never",
3639
"build:linux": "electron-vite build && node scripts/bundle-pimono-extension.mjs && electron-builder --linux --config electron-builder.config.mjs --publish never",
40+
"pretest": "node scripts/check-node-version.mjs",
3741
"test": "vitest run",
3842
"test:watch": "vitest",
3943
"fixtures:audio": "node scripts/gen-audio-fixtures.mjs",
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Fail the test run early, and legibly, when Node is outside the supported range.
2+
//
3+
// Without this the failure is silent and deeply confusing: on Node >= 24 the
4+
// runtime ships its own experimental `localStorage` global that is `undefined`
5+
// unless `--localstorage-file` is passed, and it takes precedence over the one
6+
// jsdom installs. Every `// @vitest-environment jsdom` suite that touches
7+
// localStorage then dies with `Cannot read properties of undefined (reading
8+
// 'clear')` — 29 test files and 129 tests on Node 26.3.1, all green on Node
9+
// 22.16.0 with the same code and the same lockfile. CI pins Node 22, so the
10+
// breakage is invisible there and only ever hits a contributor's machine.
11+
//
12+
// The floor is not cosmetic either: `@earendil-works/pi-ai` declares
13+
// `engines.node >= 22.19.0`, so an older 22.x installs but is unsupported.
14+
//
15+
// Kept as a `pretest` hook rather than `engine-strict=true` in .npmrc: that flag
16+
// makes pnpm enforce the engines field of every transitive dependency, which is a
17+
// far larger blast radius than the problem this guards.
18+
19+
const RANGE = '>=22.19.0 <23'
20+
21+
const [major, minor] = process.versions.node.split('.').map(Number)
22+
const supported = major === 22 && minor >= 19
23+
24+
if (!supported) {
25+
const why =
26+
major >= 24
27+
? "Node >= 24 provides its own `localStorage` global that shadows jsdom's and is undefined " +
28+
'without --localstorage-file, so the jsdom test suites fail with confusing undefined errors.'
29+
: 'This project depends on packages that require Node >= 22.19.0.'
30+
31+
process.stderr.write(
32+
`\nUnsupported Node version for the Omi Windows desktop test suite.\n\n` +
33+
` required: ${RANGE} (package.json "engines.node", and what CI pins)\n` +
34+
` running: v${process.versions.node}\n\n` +
35+
`${why}\n\n` +
36+
`Switch with your version manager, e.g. \`nvm use 22\` or \`fnm use 22\`, then re-run.\n\n`
37+
)
38+
process.exit(1)
39+
}

0 commit comments

Comments
 (0)