useStorage Audit

[AndreyYolkin]

Description

An audit of the useStorage composable has revealed several critical issues related to data integrity, reference safety, and API semantics that could lead to bugs in production environments.

Critical Issues (P1)

1. Shared Reference Mutation in defaultValue

When get(key, defaultValue) is called and the key is absent in storage, the defaultValue is assigned directly to the reactive ref. If defaultValue is an object or array, any mutation to ref.value will mutate the original defaultValue object.

• Impact: Multiple components using the same default object for different keys will inadvertently share state.
• Recommended Fix: Implement protective cloning of defaultValue when initializing the ref.

const defaults = { theme: 'light', flags: [] }
const a = storage.get('a', defaults)
const b = storage.get('b', defaults)

a.value.theme = 'dark'

2. Data Loss for Empty String Values

In readStored, the check if (!raw) return undefined incorrectly treats an empty string "" as an absent value.

• Impact: Legitimate empty string values stored by custom serializers are ignored and replaced by the default value.
• Recommended Fix: Change the check to if (raw == null).

Semantic Issues (P2)

3. Inconsistent has/remove/clear Behavior

Current implementations of has, remove, and clear only interact with the local cache (keys already accessed via get) rather than the underlying adapter storage.

• Example: storage.has('key') returns false even if the key exists in localStorage, provided it hasn't been accessed in the current session yet.
• Impact: Unpredictable behavior for developers expecting standard storage-like semantics.
• Recommended Fix: Align these methods with the adapter's physical state or rename them to reflect their cache-only scope.

4. TTL Object Wrapping Transparency

When ttl is enabled, values are wrapped in an internal { __ttl, __v, __t } structure before being passed to the serializer.write method.

• Impact: Custom serializers might fail if they expect the raw value type.
• Recommended Fix: Either unwrap the value before serialization or explicitly document this internal structure for custom serializer authors.

Environment & Sync Issues (P3)

5. Restricted Cross-Tab Synchronization

Cross-tab sync is hardcoded to work only with window.localStorage and only for already cached keys.

• Impact: No synchronization for custom adapters (e.g., indexedDB-based).
• Recommended Fix: Abstract the sync mechanism into the adapter interface or provide an event-driven sync option.

Proposed Action Plan

1. Fix defaultValue cloning and empty string handling (P1).
2. Update has/remove/clear to check the adapter state (P2).
3. Update documentation to clarify TTL wrapping and object support limitations (P2).
4. Expand test suite to cover cyclic references and cross-adapter sync edge cases (P3).

[johnleider]

Fixed 1, 2, and 3 in 21f0f3c.

Re: #4 — TTL envelope leaking into custom serializers

Right now the TTL wrapper ({ __ttl, __v, __t }) gets built before hitting serializer.write, so custom serializers see the envelope instead of the raw value. For the default JSON serializer this is invisible, but anyone writing a custom serializer that expects e.g. a plain string will get a surprise object. Worth documenting that custom serializers must handle the envelope when TTL is enabled — or we could restructure so wrapping happens after serialization, but that's a format change.

Re: #5 — Cross-tab sync only works with localStorage

This is correct by design — the storage event only fires for localStorage per spec, so sessionStorage and custom adapters can't sync this way. If we want sync for custom adapters (indexedDB, etc.), the adapter interface would need an optional subscribe method or similar. Not urgent, but something to think about if we ever ship an indexedDB adapter. The handler also re-reads from the adapter via readStored(e.key) instead of parsing e.newValue directly — minor inefficiency we could clean up.