fix(test): restore jsdom Web Storage in UI vitest setups on Node 25+#7603
Closed
davion-knight wants to merge 1 commit into
Closed
fix(test): restore jsdom Web Storage in UI vitest setups on Node 25+#7603davion-knight wants to merge 1 commit into
davion-knight wants to merge 1 commit into
Conversation
Node 25 shipped a native globalThis.localStorage that evaluates to undefined unless --localstorage-file is passed (nodejs/node#60303, still open). Vitest's jsdom environment copies a window key onto the test global only when that key is in its static allowlist or not already present; localStorage and sessionStorage are in neither, so on Node <=24 jsdom's real Storage is copied over, but on Node 25+ the key already exists as Node's broken accessor and the copy is skipped. Every localStorage-touching suite in loopover-ui and loopover-miner-ui then throws on its first .clear()/.setItem(). Restore jsdom's Storage from the raw JSDOM instance vitest exposes as globalThis.jsdom, which needs no Node CLI flags -- the flag workarounds are unrecognized by the Node 22 that .nvmrc pins, so they would break CI. Uses Object.defineProperty rather than assignment because the DOM lib types localStorage as read-only. A strict no-op on Node <=24, where the key is absent and jsdom's Storage was already copied. Adds a canary test so a future .nvmrc bump past Node 24 fails on one explicit assertion instead of every storage-touching suite throwing opaquely. Closes JSONbored#7576
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
JSONbored
reviewed
Jul 21, 2026
JSONbored
left a comment
Owner
There was a problem hiding this comment.
Merge conflicts, closing:
This branch has conflicts that must be resolved
Use the [web editor](https://github.com/JSONbored/loopover/pull/7603/conflicts) or the command line to resolve conflicts before continuing.
apps/loopover-miner-ui/vitest.setup.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Node 25 shipped a native
globalThis.localStoragethat evaluates toundefinedunless--localstorage-fileis passed (nodejs/node#60303, still open). Vitest's jsdom environment copies a window key onto the test global only when that key is in its static allowlist or not already present —localStorage/sessionStorageare in neither. So on Node ≤24 jsdom's realStorageis copied over, but on Node 25+ the key already exists (Node's broken accessor) and the copy is skipped, leavingundefined.Result: all 6 storage-touching suites in
loopover-uiandtheme-toggle.test.tsxinloopover-miner-uithrowCannot read properties of undefined (reading 'clear'). Reported upstream as vitest-dev/vitest#8757 and closed as "non-LTS is not supported", so there is no upstream fix to wait for..nvmrcpins Node 22, so CI is unaffected today — this is a make-local-dev-work fix that becomes CI-breaking the moment.nvmrcmoves past Node 24.Fix
Restore jsdom's Storage from the raw JSDOM instance vitest exposes as
globalThis.jsdom— no Node CLI flags (the flag workarounds are unrecognized by the pinned Node 22, so they would break CI). UsesObject.definePropertyrather than direct assignment, since the DOM lib typeslocalStorageas read-only. A strict no-op on Node ≤24, where the key is absent and jsdom's Storage was already copied.Verification
Node 22 is what
.nvmrcpins and what I have locally, so the broken path can't reproduce here. Instead:undefined, jsdom Storage onglobalThis.jsdom) and ran the shipped loop against it: it restores workinglocalStorage/sessionStorage(setItem/getItem/clear all functional) on the 25+ condition and leaves an already-copied Storage untouched on the ≤24 condition.ui:typecheckexits 0;ui:lintreports 0 errors; the twoloopover-miner-uitypecheck errors are pre-existing (verified identical with these changes stashed).Canary test
apps/loopover-ui/src/lib/jsdom-web-storage.test.tsasserts the jsdom environment exposes workinglocalStorage/sessionStorageand that the bare global resolves to the same object as the window property. It passes trivially on Node 22; its job is to fail on one explicit assertion the moment.nvmrcmoves past Node 24 with this shim removed, instead of every storage-touching suite failing opaquely.apps/**is outside Codecov's scope, so nocodecov/patchobligation. Test infrastructure only — no visual change.Closes #7576