refactor(frontend): wrap UserDraft localStorage payload as { value }

localStorage entries now look like {"value": <draft>} instead of just
<draft>. The wrapping is invisible at the API boundary — UserDraft.use,
.save, .get, .remove all still operate on the unwrapped draft value —
but it leaves room to add metadata (timestamps, originating user,
schema version, ...) later without breaking existing entries.

Internals:
- StoredDraft<V> = { value: V } is what we serialise to localStorage
  and what useLocalStorageValue's $state holds.
- wrap()/unwrap() helpers gate the boundary; the handle returned by
  use() unwraps on get and wraps on set.
- readPersisted() defensively drops entries whose payload isn't a
  { value: ... } object, so pre-migration drafts written by earlier
  commits on this branch are simply ignored (has() returns false,
  get() returns undefined) rather than confusingly surfacing as
  undefined-shaped drafts.

Test data switched from { value: X } (which collides confusingly with
the wrapper shape) to plain primitives / objects, plus a regression
test for the pre-migration ignore behaviour. 28 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Diego Imbert
2026-05-12 21:31:16 +02:00
parent fd2e243795
commit f50971a8d5
2 changed files with 155 additions and 146 deletions
+48 -29
View File
@@ -42,7 +42,14 @@ export type UserDraftUseOptions<V> = UserDraftOptions & {
defaultValue?: V
}
type DraftState<V> = { val: V | undefined }
/**
* The shape of what we actually persist. Wrapping the value lets us add
* metadata (timestamps, originating user, schema version, ...) later
* without breaking existing entries.
*/
type StoredDraft<V> = { value: V }
type DraftState<V> = { val: StoredDraft<V> | undefined }
type DraftEntry = {
count: number
@@ -71,13 +78,37 @@ function isLocalOnly(path: string): boolean {
return path === ''
}
function createInMemoryState<T>(defaultValue: T | undefined): DraftState<T> {
let s = $state<T | undefined>(defaultValue)
function wrap<V>(value: V | undefined): StoredDraft<V> | undefined {
return value === undefined ? undefined : { value }
}
function unwrap<V>(stored: StoredDraft<V> | undefined): V | undefined {
return stored?.value
}
function readPersisted<V>(key: string): StoredDraft<V> | undefined {
try {
const raw = localStorage.getItem(key)
if (raw == null || raw === 'undefined') return undefined
const parsed = JSON.parse(raw)
// Defensive: drop entries written before the wrapping migration. Their
// raw payload doesn't have a `.value` and would surface as undefined
// anyway — we just don't want to confuse `has()` callers.
if (parsed == null || typeof parsed !== 'object' || !('value' in parsed)) return undefined
return parsed as StoredDraft<V>
} catch (e) {
console.error('UserDraft: localStorage read failed', e)
return undefined
}
}
function createInMemoryState<V>(defaultValue: StoredDraft<V> | undefined): DraftState<V> {
let s = $state<StoredDraft<V> | undefined>(defaultValue)
return {
get val(): T | undefined {
get val(): StoredDraft<V> | undefined {
return s
},
set val(newVal: T | undefined) {
set val(newVal: StoredDraft<V> | undefined) {
s = newVal
}
}
@@ -104,13 +135,13 @@ export const UserDraft = {
if (entry) {
// Update the shared reactive state so all observers are notified.
// For non-empty paths the underlying useLocalStorageValue setter
// persists; for empty paths the in-memory state stays in-memory.
entry.state.val = value
// persists the wrapped value; for empty paths it stays in-memory.
entry.state.val = wrap(value)
return
}
if (isLocalOnly(path)) return
try {
localStorage.setItem(localStorageKey(ws, itemKind, path), JSON.stringify(value))
localStorage.setItem(localStorageKey(ws, itemKind, path), JSON.stringify(wrap(value)))
} catch (e) {
console.error('UserDraft.save: localStorage write failed', e)
}
@@ -125,17 +156,10 @@ export const UserDraft = {
const mk = mapKey(ws, itemKind, path)
const entry = entries.get(mk)
if (entry) {
return entry.state.val as V | undefined
return unwrap(entry.state.val as StoredDraft<V> | undefined)
}
if (isLocalOnly(path)) return undefined
try {
const raw = localStorage.getItem(localStorageKey(ws, itemKind, path))
if (raw == null || raw === 'undefined') return undefined
return JSON.parse(raw) as V
} catch (e) {
console.error('UserDraft.get: localStorage read failed', e)
return undefined
}
return unwrap(readPersisted<V>(localStorageKey(ws, itemKind, path)))
},
/**
@@ -150,12 +174,7 @@ export const UserDraft = {
const entry = entries.get(mk)
if (entry) return entry.state.val !== undefined
if (isLocalOnly(path)) return false
try {
const raw = localStorage.getItem(localStorageKey(ws, itemKind, path))
return raw != null && raw !== 'undefined'
} catch {
return false
}
return readPersisted(localStorageKey(ws, itemKind, path)) !== undefined
},
remove(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void {
@@ -174,15 +193,15 @@ export const UserDraft = {
): UserDraftHandle<V> {
const ws = resolveWorkspace(opts)
const mk = mapKey(ws, itemKind, path)
const defaultValue = opts?.defaultValue
const wrappedDefault = wrap(opts?.defaultValue)
let entry = entries.get(mk)
if (!entry) {
const state: DraftState<unknown> = isLocalOnly(path)
? createInMemoryState<unknown>(defaultValue)
: useLocalStorageValue<unknown>(
? createInMemoryState<unknown>(wrappedDefault)
: useLocalStorageValue<StoredDraft<unknown> | undefined>(
localStorageKey(ws, itemKind, path),
defaultValue,
wrappedDefault,
undefined,
// The first value to flow into the handle (e.g. a backend load
// in the editor route) is the baseline — only persist when the
@@ -208,12 +227,12 @@ export const UserDraft = {
return {
get draft(): V | undefined {
return sharedEntry.state.val as V | undefined
return unwrap(sharedEntry.state.val as StoredDraft<V> | undefined)
},
set draft(value: V | undefined) {
// useLocalStorageValue's setter writes synchronously and
// removes the localStorage entry when value is undefined.
sharedEntry.state.val = value
sharedEntry.state.val = wrap(value)
}
}
}
+107 -117
View File
@@ -23,6 +23,12 @@ function flushDestroyCallbacks(): void {
for (const cb of callbacks) cb()
}
// Helper: localStorage payloads are always wrapped as { value: <draft> } so
// future metadata fields can be added without breaking existing entries.
function wrapped<V>(value: V): string {
return JSON.stringify({ value })
}
beforeEach(() => {
__resetUserDraftForTesting()
onDestroyCallbacks.length = 0
@@ -31,33 +37,39 @@ beforeEach(() => {
})
describe('UserDraft.save / get / remove (no observers)', () => {
it('save writes to localStorage under the workspace-scoped key', () => {
UserDraft.save('flow', 'u/me/myflow', { value: { hello: 'world' } })
it('save writes a wrapped { value } payload under the workspace-scoped key', () => {
UserDraft.save('flow', 'u/me/myflow', { hello: 'world' })
const raw = localStorage.getItem('userdraft/w/test_ws/flow/u/me/myflow')
expect(raw).toBe(JSON.stringify({ value: { hello: 'world' } }))
expect(raw).toBe(wrapped({ hello: 'world' }))
})
it('get reads from localStorage when no observer is registered', () => {
localStorage.setItem(
'userdraft/w/test_ws/script/u/me/script1',
JSON.stringify({ value: 'code' })
)
it('get reads from a wrapped localStorage payload when no observer is registered', () => {
localStorage.setItem('userdraft/w/test_ws/script/u/me/script1', wrapped('code'))
expect(UserDraft.get('script', 'u/me/script1')).toEqual({ value: 'code' })
expect(UserDraft.get('script', 'u/me/script1')).toBe('code')
})
it('get returns undefined when nothing is stored', () => {
expect(UserDraft.get('flow', 'u/me/missing')).toBeUndefined()
})
it('get returns undefined when stored value is malformed', () => {
it('get returns undefined when the stored payload is malformed', () => {
localStorage.setItem('userdraft/w/test_ws/flow/u/me/bad', 'not-json')
expect(UserDraft.get('flow', 'u/me/bad')).toBeUndefined()
})
it('get returns undefined when the stored payload is unwrapped (pre-migration entry)', () => {
// Drafts written before the wrapping was introduced look like the raw
// value rather than { value: ... }. They must be ignored rather than
// surface as undefined-shaped drafts.
localStorage.setItem('userdraft/w/test_ws/flow/u/me/raw', JSON.stringify({ hello: 'world' }))
expect(UserDraft.get('flow', 'u/me/raw')).toBeUndefined()
expect(UserDraft.has('flow', 'u/me/raw')).toBe(false)
})
it('remove clears the localStorage entry', () => {
UserDraft.save('app', 'u/me/app1', { value: { grid: [] } })
UserDraft.save('app', 'u/me/app1', { grid: [] })
expect(localStorage.getItem('userdraft/w/test_ws/app/u/me/app1')).not.toBeNull()
UserDraft.remove('app', 'u/me/app1')
@@ -65,100 +77,91 @@ describe('UserDraft.save / get / remove (no observers)', () => {
})
it('uses the workspace from opts when provided', () => {
UserDraft.save('flow', 'u/me/f', { value: 1 }, { workspace: 'other_ws' })
UserDraft.save('flow', 'u/me/f', 1, { workspace: 'other_ws' })
expect(localStorage.getItem('userdraft/w/other_ws/flow/u/me/f')).toBe(
JSON.stringify({ value: 1 })
)
expect(localStorage.getItem('userdraft/w/other_ws/flow/u/me/f')).toBe(wrapped(1))
// Default workspace key must remain empty.
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/f')).toBeNull()
})
it('supports trigger kinds as item kinds', () => {
UserDraft.save('trigger_kafka', 'u/me/topic1', { value: { brokers: ['localhost:9092'] } })
UserDraft.save('trigger_kafka', 'u/me/topic1', { brokers: ['localhost:9092'] })
const raw = localStorage.getItem('userdraft/w/test_ws/trigger_kafka/u/me/topic1')
expect(raw).toBe(JSON.stringify({ value: { brokers: ['localhost:9092'] } }))
expect(raw).toBe(wrapped({ brokers: ['localhost:9092'] }))
})
it('throws when neither opts.workspace nor $workspaceStore is set', () => {
workspaceStore.set(undefined)
expect(() => UserDraft.save('flow', 'u/me/x', { value: 1 })).toThrow(/no workspace/)
expect(() => UserDraft.save('flow', 'u/me/x', 1)).toThrow(/no workspace/)
})
})
describe('UserDraft.use() — observer sync', () => {
it('loads the existing localStorage value on first use', () => {
localStorage.setItem(
'userdraft/w/test_ws/flow/u/me/loaded',
JSON.stringify({ value: 'preloaded' })
)
localStorage.setItem('userdraft/w/test_ws/flow/u/me/loaded', wrapped('preloaded'))
const handle = UserDraft.use<{ value: string }>('flow', 'u/me/loaded')
expect(handle.draft).toEqual({ value: 'preloaded' })
const handle = UserDraft.use<string>('flow', 'u/me/loaded')
expect(handle.draft).toBe('preloaded')
})
it('two handles on the same key share the same underlying state', () => {
const a = UserDraft.use<{ value: number }>('flow', 'u/me/shared')
const b = UserDraft.use<{ value: number }>('flow', 'u/me/shared')
const a = UserDraft.use<number>('flow', 'u/me/shared')
const b = UserDraft.use<number>('flow', 'u/me/shared')
a.draft = { value: 42 }
expect(b.draft).toEqual({ value: 42 })
a.draft = 42
expect(b.draft).toBe(42)
b.draft = { value: 99 }
expect(a.draft).toEqual({ value: 99 })
b.draft = 99
expect(a.draft).toBe(99)
})
it('save() propagates to live use() handles (in-memory)', () => {
const handle = UserDraft.use<{ value: number }>('flow', 'u/me/observed')
const handle = UserDraft.use<number>('flow', 'u/me/observed')
expect(handle.draft).toBeUndefined()
// First write through a live entry is treated as the "initial value"
// (saveInitialValue=false) and is NOT persisted — observers still see it.
UserDraft.save('flow', 'u/me/observed', { value: 7 })
expect(handle.draft).toEqual({ value: 7 })
UserDraft.save('flow', 'u/me/observed', 7)
expect(handle.draft).toBe(7)
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/observed')).toBeNull()
// Subsequent writes persist.
UserDraft.save('flow', 'u/me/observed', { value: 9 })
expect(handle.draft).toEqual({ value: 9 })
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/observed')).toBe(
JSON.stringify({ value: 9 })
)
UserDraft.save('flow', 'u/me/observed', 9)
expect(handle.draft).toBe(9)
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/observed')).toBe(wrapped(9))
})
it('remove() clears localStorage without touching the in-memory handle', () => {
// Seed localStorage so the live handle initialises from it.
localStorage.setItem('userdraft/w/test_ws/flow/u/me/removed', JSON.stringify({ value: 1 }))
const handle = UserDraft.use<{ value: number }>('flow', 'u/me/removed')
expect(handle.draft).toEqual({ value: 1 })
localStorage.setItem('userdraft/w/test_ws/flow/u/me/removed', wrapped(1))
const handle = UserDraft.use<number>('flow', 'u/me/removed')
expect(handle.draft).toBe(1)
UserDraft.remove('flow', 'u/me/removed')
// Live handle keeps its current value — remove() only wipes the
// persisted side. This is what lets callers run UserDraft.remove
// during navigation without flickering the editor UI.
expect(handle.draft).toEqual({ value: 1 })
expect(handle.draft).toBe(1)
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/removed')).toBeNull()
})
it('the second write through the handle setter persists to localStorage', () => {
const handle = UserDraft.use<{ value: string }>('flow', 'u/me/setter')
const handle = UserDraft.use<string>('flow', 'u/me/setter')
// First write is the baseline — not persisted.
handle.draft = { value: 'initial' }
handle.draft = 'initial'
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/setter')).toBeNull()
// Second (and onwards) persists.
handle.draft = { value: 'persisted' }
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/setter')).toBe(
JSON.stringify({ value: 'persisted' })
)
handle.draft = 'persisted'
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/setter')).toBe(wrapped('persisted'))
})
it('setting handle.draft = undefined after edits removes the localStorage entry', () => {
const handle = UserDraft.use<{ value: string }>('flow', 'u/me/clear')
handle.draft = { value: 'initial' } // baseline, not persisted
handle.draft = { value: 'edited' } // persisted
const handle = UserDraft.use<string>('flow', 'u/me/clear')
handle.draft = 'initial' // baseline, not persisted
handle.draft = 'edited' // persisted
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/clear')).not.toBeNull()
handle.draft = undefined
@@ -167,118 +170,109 @@ describe('UserDraft.use() — observer sync', () => {
})
it('two handles in different workspaces are isolated', () => {
const a = UserDraft.use<{ value: number }>('flow', 'u/me/iso', { workspace: 'ws_a' })
const b = UserDraft.use<{ value: number }>('flow', 'u/me/iso', { workspace: 'ws_b' })
const a = UserDraft.use<number>('flow', 'u/me/iso', { workspace: 'ws_a' })
const b = UserDraft.use<number>('flow', 'u/me/iso', { workspace: 'ws_b' })
a.draft = { value: 1 }
b.draft = { value: 2 }
a.draft = 1
b.draft = 2
expect(a.draft).toEqual({ value: 1 })
expect(b.draft).toEqual({ value: 2 })
expect(a.draft).toBe(1)
expect(b.draft).toBe(2)
})
it('save() falls back to localStorage when no handle is registered', () => {
UserDraft.save('flow', 'u/me/noobs', { value: 'fallback' })
UserDraft.save('flow', 'u/me/noobs', 'fallback')
// First use() afterwards loads the persisted value.
const handle = UserDraft.use<{ value: string }>('flow', 'u/me/noobs')
expect(handle.draft).toEqual({ value: 'fallback' })
const handle = UserDraft.use<string>('flow', 'u/me/noobs')
expect(handle.draft).toBe('fallback')
})
})
describe('UserDraft.use() — defaultValue', () => {
it('returns defaultValue when localStorage has no entry', () => {
const handle = UserDraft.use<{ value: string }>('flow', 'u/me/withdefault', {
defaultValue: { value: 'fallback' }
})
const handle = UserDraft.use<string>('flow', 'u/me/withdefault', { defaultValue: 'fallback' })
expect(handle.draft).toEqual({ value: 'fallback' })
expect(handle.draft).toBe('fallback')
})
it('does not persist the defaultValue on first read', () => {
UserDraft.use<{ value: string }>('flow', 'u/me/lazyDefault', {
defaultValue: { value: 'fallback' }
})
UserDraft.use<string>('flow', 'u/me/lazyDefault', { defaultValue: 'fallback' })
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/lazyDefault')).toBeNull()
})
it('localStorage value wins over defaultValue', () => {
localStorage.setItem(
'userdraft/w/test_ws/flow/u/me/overridden',
JSON.stringify({ value: 'persisted' })
)
localStorage.setItem('userdraft/w/test_ws/flow/u/me/overridden', wrapped('persisted'))
const handle = UserDraft.use<{ value: string }>('flow', 'u/me/overridden', {
defaultValue: { value: 'fallback' }
const handle = UserDraft.use<string>('flow', 'u/me/overridden', {
defaultValue: 'fallback'
})
expect(handle.draft).toEqual({ value: 'persisted' })
expect(handle.draft).toBe('persisted')
})
it('second write through the setter persists even though defaultValue was set', () => {
const handle = UserDraft.use<{ value: string }>('flow', 'u/me/writeDefault', {
defaultValue: { value: 'fallback' }
const handle = UserDraft.use<string>('flow', 'u/me/writeDefault', {
defaultValue: 'fallback'
})
// First write is the initial-value baseline.
handle.draft = { value: 'initial' }
handle.draft = 'initial'
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/writeDefault')).toBeNull()
handle.draft = { value: 'modified' }
handle.draft = 'modified'
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/writeDefault')).toBe(
JSON.stringify({ value: 'modified' })
wrapped('modified')
)
})
})
describe('UserDraft — empty path (new-item, in-memory only)', () => {
it('use() with empty path uses defaultValue and does not persist', () => {
const handle = UserDraft.use<{ value: number }>('flow', '', {
defaultValue: { value: 0 }
})
const handle = UserDraft.use<number>('flow', '', { defaultValue: 0 })
handle.draft = { value: 99 }
handle.draft = 99
expect(handle.draft).toEqual({ value: 99 })
expect(handle.draft).toBe(99)
// No localStorage key with empty path should ever be written.
expect(localStorage.getItem('userdraft/w/test_ws/flow/')).toBeNull()
})
it('two handles with empty path share in-memory state per workspace', () => {
const a = UserDraft.use<{ value: number }>('flow', '')
const b = UserDraft.use<{ value: number }>('flow', '')
const a = UserDraft.use<number>('flow', '')
const b = UserDraft.use<number>('flow', '')
a.draft = { value: 1 }
expect(b.draft).toEqual({ value: 1 })
a.draft = 1
expect(b.draft).toBe(1)
b.draft = { value: 2 }
expect(a.draft).toEqual({ value: 2 })
b.draft = 2
expect(a.draft).toBe(2)
})
it('save() with empty path is a no-op against localStorage but updates live handles', () => {
const handle = UserDraft.use<{ value: number }>('flow', '')
const handle = UserDraft.use<number>('flow', '')
UserDraft.save('flow', '', { value: 5 })
expect(handle.draft).toEqual({ value: 5 })
UserDraft.save('flow', '', 5)
expect(handle.draft).toBe(5)
expect(localStorage.getItem('userdraft/w/test_ws/flow/')).toBeNull()
})
it('get() with empty path returns the in-memory value when a handle is live, else undefined', () => {
expect(UserDraft.get('flow', '')).toBeUndefined()
const handle = UserDraft.use<{ value: number }>('flow', '')
handle.draft = { value: 11 }
expect(UserDraft.get('flow', '')).toEqual({ value: 11 })
const handle = UserDraft.use<number>('flow', '')
handle.draft = 11
expect(UserDraft.get('flow', '')).toBe(11)
})
it('remove() with empty path is a no-op (no localStorage to clear, in-memory untouched)', () => {
const handle = UserDraft.use<{ value: number }>('flow', '')
handle.draft = { value: 1 }
const handle = UserDraft.use<number>('flow', '')
handle.draft = 1
UserDraft.remove('flow', '')
// Empty-path entries never touched localStorage and remove() no longer
// resets the in-memory state, so the handle keeps its value.
expect(handle.draft).toEqual({ value: 1 })
expect(handle.draft).toBe(1)
expect(localStorage.getItem('userdraft/w/test_ws/flow/')).toBeNull()
})
})
@@ -286,12 +280,12 @@ describe('UserDraft — empty path (new-item, in-memory only)', () => {
describe('UserDraft.use() — reference counting & cleanup', () => {
it('destroys the entry when the last handle is released', () => {
// First handle acquires the entry.
const a = UserDraft.use<{ value: number }>('flow', 'u/me/ref')
a.draft = { value: 1 } // baseline write — not persisted
const a = UserDraft.use<number>('flow', 'u/me/ref')
a.draft = 1 // baseline write — not persisted
// Second handle increments the count.
const b = UserDraft.use<{ value: number }>('flow', 'u/me/ref')
expect(b.draft).toEqual({ value: 1 })
const b = UserDraft.use<number>('flow', 'u/me/ref')
expect(b.draft).toBe(1)
// onDestroy for both handles got registered.
expect(onDestroyCallbacks.length).toBe(2)
@@ -300,12 +294,10 @@ describe('UserDraft.use() — reference counting & cleanup', () => {
const firstCb = onDestroyCallbacks.shift()!
firstCb()
UserDraft.save('flow', 'u/me/ref', { value: 2 })
expect(a.draft).toEqual({ value: 2 })
UserDraft.save('flow', 'u/me/ref', 2)
expect(a.draft).toBe(2)
// Now persisted (second write after the baseline).
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/ref')).toBe(
JSON.stringify({ value: 2 })
)
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/ref')).toBe(wrapped(2))
// Releasing the second handle drops the entry; subsequent save()
// must go straight to localStorage rather than mutating in-memory
@@ -313,21 +305,19 @@ describe('UserDraft.use() — reference counting & cleanup', () => {
const secondCb = onDestroyCallbacks.shift()!
secondCb()
UserDraft.save('flow', 'u/me/ref', { value: 3 })
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/ref')).toBe(
JSON.stringify({ value: 3 })
)
UserDraft.save('flow', 'u/me/ref', 3)
expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/ref')).toBe(wrapped(3))
})
it('a fresh use() after cleanup re-reads the latest persisted value', () => {
const a = UserDraft.use<{ value: string }>('flow', 'u/me/cycle')
a.draft = { value: 'initial' } // baseline — not persisted
a.draft = { value: 'edited' } // persisted
const a = UserDraft.use<string>('flow', 'u/me/cycle')
a.draft = 'initial' // baseline — not persisted
a.draft = 'edited' // persisted
flushDestroyCallbacks()
// After all handles release, a brand-new use() must pick up the
// value persisted to localStorage from the previous round.
const b = UserDraft.use<{ value: string }>('flow', 'u/me/cycle')
expect(b.draft).toEqual({ value: 'edited' })
const b = UserDraft.use<string>('flow', 'u/me/cycle')
expect(b.draft).toBe('edited')
})
})