Fix tests for node 26 - #12401
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to address Vitest failures on newer Node versions (notably Node 26) by ensuring a usable localStorage backing store in the test runtime and aligning the in-memory Storage fallback behavior with expectations used throughout the codebase.
Changes:
- Configure Vitest’s Node process arguments to supply a temp
--localstorage-filepath for the jsdom test environment. - Adjust the
core/preferencesin-memory storage fallback to returnnull(instead ofundefined) for missing keys.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
vitest.config.ts |
Adds Node exec args to provide a temp localStorage file for tests under newer Node versions. |
modules/core/preferences.js |
Tweaks the in-memory Storage fallback to better match localStorage semantics for missing keys. |
tyrasd
left a comment
There was a problem hiding this comment.
🤔
From what I can see, the crashes are actually coming from osm-auth which does not properly fall back to a polyfill when the localStorage object is undefined. See osmlab/osm-auth#152 for a proper fix for that.
Until then we could do the workaround of hacking1 our polyfill into the globalThis object, or alternatively we could just disable the experimental webstorage support in vitest.config.ts:
execArgv: [
'--no-experimental-webstorage'
],
Which would be slightly nicer, as then we'd also not get the false positive warnings about missing --local-storage-file parameter in the logs.
What do you think?
Footnotes
-
which could be slightly dangerous as we only implement the subset of the API we need, but others might actually call, say
localStorage.lengthpotentially. ↩
Closes #12047.
When running
brew install node(defaults to v26) followed bynpm testI get quite a few failures.let s = {}) is flawed since all of our tests expectnull, notundefinedif an item doesn't exist in storage.