From e23c7374e11b87101caffbe70443a839d4700619 Mon Sep 17 00:00:00 2001 From: ghzhost Date: Wed, 2 Sep 2026 09:16:59 +0000 Subject: [PATCH] fix(hooks): catch storage write/remove errors gracefully in useLocalStorage (#5) - Wrap localStorage.setItem and localStorage.removeItem in try/catch blocks - Log warning to console on storage failure while keeping in-memory state functional - Use useCallback for stable set/remove handlers across renders - Add comprehensive unit tests covering read fallback, set, quota exceeded, and removal --- src/hooks/useLocalStorage.test.ts | 93 +++++++++++++++++++++++++++++++ src/hooks/useLocalStorage.ts | 36 ++++++++++-- 2 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 src/hooks/useLocalStorage.test.ts diff --git a/src/hooks/useLocalStorage.test.ts b/src/hooks/useLocalStorage.test.ts new file mode 100644 index 0000000..7c0ca9b --- /dev/null +++ b/src/hooks/useLocalStorage.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook, act } from '@testing-library/react' +import { useLocalStorage } from './useLocalStorage' + +describe('useLocalStorage', () => { + beforeEach(() => { + localStorage.clear() + vi.restoreAllMocks() + }) + + it('reads initial value from localStorage if present', () => { + localStorage.setItem('test-key', JSON.stringify({ name: 'Alice' })) + const { result } = renderHook(() => useLocalStorage('test-key', { name: 'Default' })) + expect(result.current[0]).toEqual({ name: 'Alice' }) + }) + + it('falls back to default value if key is not in localStorage or parse fails', () => { + localStorage.setItem('corrupt-key', 'invalid json{') + const { result: r1 } = renderHook(() => useLocalStorage('non-existent', 'fallback')) + expect(r1.current[0]).toBe('fallback') + + const { result: r2 } = renderHook(() => useLocalStorage('corrupt-key', 'fallback')) + expect(r2.current[0]).toBe('fallback') + }) + + it('persists value updates to localStorage and updates state', () => { + const { result } = renderHook(() => useLocalStorage('test-key', 'initial')) + + act(() => { + result.current[1]('updated') + }) + + expect(result.current[0]).toBe('updated') + expect(JSON.parse(localStorage.getItem('test-key')!)).toBe('updated') + + act(() => { + result.current[1](prev => prev + '-fn') + }) + + expect(result.current[0]).toBe('updated-fn') + expect(JSON.parse(localStorage.getItem('test-key')!)).toBe('updated-fn') + }) + + it('handles localStorage.setItem throwing QuotaExceededError gracefully without throwing', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const { result } = renderHook(() => useLocalStorage('quota-key', 'safe')) + + vi.spyOn(localStorage, 'setItem').mockImplementation(() => { + const err = new Error('QuotaExceededError') + err.name = 'QuotaExceededError' + throw err + }) + + expect(() => { + act(() => { + result.current[1]('new-value') + }) + }).not.toThrow() + + expect(result.current[0]).toBe('new-value') + expect(warnSpy).toHaveBeenCalled() + }) + + it('clears localStorage and resets state on remove()', () => { + localStorage.setItem('test-key', JSON.stringify('existing')) + const { result } = renderHook(() => useLocalStorage('test-key', 'default')) + + act(() => { + result.current[2]() + }) + + expect(result.current[0]).toBe('default') + expect(localStorage.getItem('test-key')).toBeNull() + }) + + it('handles localStorage.removeItem throwing gracefully without throwing', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const { result } = renderHook(() => useLocalStorage('test-key', 'default')) + + vi.spyOn(localStorage, 'removeItem').mockImplementation(() => { + throw new Error('SecurityError') + }) + + expect(() => { + act(() => { + result.current[2]() + }) + }).not.toThrow() + + expect(result.current[0]).toBe('default') + expect(warnSpy).toHaveBeenCalled() + }) +}) diff --git a/src/hooks/useLocalStorage.ts b/src/hooks/useLocalStorage.ts index ed5b534..05fef07 100644 --- a/src/hooks/useLocalStorage.ts +++ b/src/hooks/useLocalStorage.ts @@ -1,7 +1,35 @@ -import { useState } from 'react' +import { useState, useCallback } from 'react' + export function useLocalStorage(key: string, init: T) { - const [val, setVal] = useState(() => { try { const i = localStorage.getItem(key); return i ? JSON.parse(i) : init } catch { return init } }) - const set = (v: T | ((p: T) => T)) => { const s = v instanceof Function ? v(val) : v; setVal(s); localStorage.setItem(key, JSON.stringify(s)) } - const remove = () => { localStorage.removeItem(key); setVal(init) } + const [val, setVal] = useState(() => { + try { + const i = localStorage.getItem(key) + return i ? JSON.parse(i) : init + } catch { + return init + } + }) + + const set = useCallback((v: T | ((p: T) => T)) => { + setVal(prev => { + const next = v instanceof Function ? v(prev) : v + try { + localStorage.setItem(key, JSON.stringify(next)) + } catch (err) { + console.warn(`[useLocalStorage] Failed to persist key "${key}":`, err) + } + return next + }) + }, [key]) + + const remove = useCallback(() => { + try { + localStorage.removeItem(key) + } catch (err) { + console.warn(`[useLocalStorage] Failed to remove key "${key}":`, err) + } + setVal(init) + }, [key, init]) + return [val, set, remove] as const }