useLocalStorage/useSessionStorage: unguarded JSON.parse and unguarded setItem (6 sites)

Verified against @stacksjs/stx@0.2.119, dist/signals.js.

useLocalStorage and useSessionStorage each parse and write storage with no error handling, on three code paths apiece. Any one of them takes down the entire client script for the page.

1. Initialiser — throws on any non-JSON value already at the key

// signals.js:3878
function useLocalStorage(key, defaultValue) {
  var stored = localStorage.getItem(key);
  var initial = stored !== null ? JSON.parse(stored) : defaultValue;   // :3880

Same at :3901 for useSessionStorage.

A plain string written by an earlier version of an app — or by any non-stx code sharing the origin — is enough. JSON.parse('list') throws SyntaxError, the composable never returns, and every statement after it in the client block is dead. There is no console hint pointing at the key.

Hit this in a real app: a key had previously held the bare string list. Migrating that page to useLocalStorage silently killed the whole script. The workaround was to burn the key name and move to <key>.v2, which is not something a storage composable should ever require.

2. Cross-tab storage handler — same parse, worse blast radius

// signals.js:3888
var handler = function(e) {
  if (e.key === key) s.set(e.newValue !== null ? JSON.parse(e.newValue) : defaultValue);
};

Same at :3908.

This one throws inside a window event listener, at an arbitrary later time, triggered by a different tab. It cannot be reproduced by loading the page, and the stack trace points into the runtime rather than at the consumer. Guarding only the initialiser would leave this live.

3. Write path — no try/catch, unlike its sibling eleven lines above

// signals.js:3882
effect(function() {
  localStorage.setItem(key, JSON.stringify(s()));
});

Same at :3904.

Compare useColorMode.persist in the same file:

// signals.js:3785
function persist(pref) { try { localStorage.setItem(storageKey, pref); } catch (e) {} }

So two composables in one file take opposite positions on identical risk. Safari private mode, QuotaExceededError, and storage-disabled embedding contexts kill every useLocalStorage consumer while every useColorMode consumer survives. The difference is undocumented.

Suggested fix

Fall back to defaultValue when a read fails, and make writes non-fatal — matching the discipline readPersisted/persist already use:

function safeParse(raw, fallback) {
  if (raw === null) return fallback
  try { return JSON.parse(raw) } catch { return fallback }
}

Apply at all six sites (:3880, :3888, :3882 and :3901, :3908, :3904). A console.warn naming the key on parse failure would turn a silent script death into a one-line diagnosis.

Happy to open a PR if the shape above looks right.

Fixed in 304b8fc.

The parse fallback is lenient rather than defaulting. A value that isn't JSON now comes back as the raw string instead of being discarded for defaultValue — so the key that held a bare list keeps reading back list, and gets rewritten as JSON on the next set. Migrating an existing key no longer requires burning the name and moving to <key>.v2. This also matches composables/use-storage.ts, whose default serializer already returned the raw string on parse failure; the runtime was aligned to the module path rather than inventing a third behaviour.

Two fixes beyond what was reported:

• The cross-tab handler was unguarded on both implementations, not just the runtime. On the module path it only bites with a custom serializer (the default one swallows its own parse errors), but the failure shape is identical — a throw inside a window listener, fired by another tab, with a stack pointing at the runtime.
• useLocalStorage's listener didn't filter on storageArea, so a sessionStorage write to the same key name clobbered the signal. useSessionStorage already filtered.

test/signals/use-storage-guards.test.ts covers all of it behaviourally against both impls. Verified each assertion fails without the fix: 11 of 11 runtime tests plus the module serializer test go red when the source is reverted.

One thing this does not address, noted since it surfaced while writing the tests: the two implementations don't share a return shape. The runtime returns a signal (s() / s.set()); the module returns a StorageRef (.value / .get() / .remove()), and only the module takes an options third argument. Same class of divergence #1710 fixed for useCookie — worth its own issue.