fix(test): restore jsdom Web Storage lost by vitest's jsdom env on Node 25+#7582
fix(test): restore jsdom Web Storage lost by vitest's jsdom env on Node 25+#7582shin-core wants to merge 1 commit into
Conversation
…de 25+ On Node 25+, `globalThis.localStorage` exists as a native accessor that evaluates to `undefined` when no `--localstorage-file` is given (nodejs/node#60303). Vitest's jsdom environment only copies jsdom's real `Storage` onto the test global when the key is absent, so once `"localStorage" in globalThis` is truthy it skips the copy and leaves Node's broken `undefined` in place -- every `apps/loopover-ui` and `apps/loopover-miner-ui` test touching window.localStorage then throws `Cannot read properties of undefined`. CI pins Node 22 via .nvmrc so it is unaffected today, but local dev on newer Node is broken. Restore jsdom's working Storage (still reachable via the raw JSDOM instance vitest exposes as `globalThis.jsdom`) in each workspace's vitest.setup.ts before any test runs. The shim is a no-op on Node 22, where jsdom's copy is already present, so CI behavior is unchanged. Closes JSONbored#7576
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-21 00:32:27 UTC
Review summary Nits — 5 non-blocking
Flagged checks (non-blocking)
Decision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
JSONbored
left a comment
There was a problem hiding this comment.
One gap: packages/loopover-ui-kit (@loopover/ui-kit, the design-system package consumed by both apps/loopover-ui and apps/loopover-miner-ui) has the identical jsdom + vitest setup shape — see packages/loopover-ui-kit/vitest.setup.ts / vitest.config.ts (added in #7495). It has no test touching localStorage/sessionStorage today, so it isn't currently failing, but it'll hit the exact same Node 25+ issue the moment one does, since it goes through the same populateGlobal() gap. Worth wiring restoreJsdomWebStorage() into that workspace too (mirroring what's already done for the other two) so the fix covers every jsdom-environment workspace in the repo, not just two of the three.
One gap: packages/loopover-ui-kit (@loopover/ui-kit, the design-system package consumed by both apps/loopover-ui and apps/loopover-miner-ui) has the identical jsdom + vitest setup shape — see packages/loopover-ui-kit/vitest.setup.ts / vitest.config.ts (added in #7495). It has no test touching localStorage/sessionStorage today, so it isn't currently failing, but it'll hit the exact same Node 25+ issue the moment one does, since it goes through the same populateGlobal() gap. Worth wiring restoreJsdomWebStorage() into that workspace too (mirroring what's already done for the other two) so the fix covers every jsdom-environment workspace in the repo, not just two of the three.
JSONbored
left a comment
There was a problem hiding this comment.
Misses the mark:
One gap: packages/loopover-ui-kit (@loopover/ui-kit, the design-system package consumed by both apps/loopover-ui and apps/loopover-miner-ui) has the identical jsdom + vitest setup shape — see packages/loopover-ui-kit/vitest.setup.ts / vitest.config.ts (added in https://github.com/JSONbored/loopover/pull/7495). It has no test touching localStorage/sessionStorage today, so it isn't currently failing, but it'll hit the exact same Node 25+ issue the moment one does, since it goes through the same populateGlobal() gap. Worth wiring restoreJsdomWebStorage() into that workspace too (mirroring what's already done for the other two) so the fix covers every jsdom-environment workspace in the repo, not just two of the three.

What
On Node 25+,
globalThis.localStorageexists as a native accessor that evaluates toundefinedwhen no--localstorage-fileis supplied (nodejs/node#60303, still open). Vitest's jsdom environment copies jsdom's window properties onto the test global only when a key is absent — so once"localStorage" in globalThisis truthy (Node 25+), it skips copying jsdom's realStorageand leaves Node's brokenundefinedin place. Everyapps/loopover-ui/apps/loopover-miner-uitest that toucheswindow.localStoragethen throwsTypeError: Cannot read properties of undefined (reading 'clear').CI pins Node 22 via
.nvmrc, so it's green today — but local dev on a newer Node is broken, and this becomes CI-breaking the moment.nvmrcis bumped past Node 24. The upstream vitest issue (vitest-dev/vitest#8757) was closed as "non-LTS not supported", so there's no upstream fix to wait for.Fix
jsdom's working
Storageis still reachable via the rawJSDOMinstance vitest exposes asglobalThis.jsdom. A tiny helper (src/test/restore-jsdom-web-storage.tsin each workspace) copieslocalStorage/sessionStoragefromglobalThis.jsdom.windowonto the global whenever the environment's own copy is missing, called from eachvitest.setup.tsbefore any test runs..nvmrc/ CI): jsdom's copy is already present (globalThis.localStorageis truthy), so the guard skips every key — CI behavior is byte-identical.jsdom.windowreference, so bothlocalStorageandwindow.localStoragework in a test.Tests
src/test/restore-jsdom-web-storage.test.tssimulates the Node 25+ broken state (a target where"localStorage" in targetis true but the value isundefined) so the fix is exercised deterministically on any Node, including CI's Node 22: it asserts the shim restores a working Storage, leaves an already-working Storage untouched (the CI no-op path), no-ops without a jsdom instance, and skips a storage jsdom doesn't expose. 4/4 pass; both full UI suites stay green (415 + 332 tests).Scope
apps/**test infrastructure only (twovitest.setup.ts+ two helpers + one test) — nosrc/**, no runtime/UI component, no visual change.Closes #7576