Merge pull request #786 from TylerR909/fix-useLocalStorage

Fix use local storage
streamich · Feb 3, 2020 · 5bfdf65 · 5bfdf65
2 parents 2e91e53 + 68fb835
commit 5bfdf65
Show file tree

Hide file tree

Showing 4 changed files with 237 additions and 80 deletions.
diff --git a/package.json b/package.json
@@ -163,8 +163,7 @@
       "<rootDir>/tests/**/*.test.(ts|tsx)"
     ],
     "setupFiles": [
-      "<rootDir>/tests/_setup.js",
-      "./tests/setupTests.ts"
+      "<rootDir>/tests/_setup.js"
     ]
   }
 }
diff --git a/src/useLocalStorage.ts b/src/useLocalStorage.ts
@@ -1,4 +1,4 @@
-import { useEffect, useState, useCallback } from 'react';
+import { useState, useCallback, Dispatch, SetStateAction } from 'react';
 import { isClient } from './util';
 
 type parserOptions<T> =
@@ -12,23 +12,25 @@ type parserOptions<T> =
     };
 
 const noop = () => {};
-const isUndefined = (value?: any): boolean => typeof value === 'undefined';
 
 const useLocalStorage = <T>(
   key: string,
   initialValue?: T,
   options?: parserOptions<T>
-): [T | undefined, React.Dispatch<React.SetStateAction<T | undefined>>, () => void] => {
+): [T | undefined, Dispatch<SetStateAction<T | undefined>>, () => void] => {
   if (!isClient) {
     return [initialValue as T, noop, noop];
   }
+  if (!key) {
+    throw new Error('useLocalStorage key may not be falsy');
+  }
 
-  // Use provided serializer/deserializer or the default ones
-  const serializer = options ? (options.raw ? String : options.serializer) : JSON.stringify;
-  const deserializer = options ? (options.raw ? String : options.deserializer) : JSON.parse;
+  const deserializer = options ? (options.raw ? value => value : options.deserializer) : JSON.parse;
 
   const [state, setState] = useState<T | undefined>(() => {
     try {
+      const serializer = options ? (options.raw ? String : options.serializer) : JSON.stringify;
+
       const localStorageValue = localStorage.getItem(key);
       if (localStorageValue !== null) {
         return deserializer(localStorageValue);
@@ -44,6 +46,31 @@ const useLocalStorage = <T>(
     }
   });
 
+  const set: Dispatch<SetStateAction<T | undefined>> = useCallback(
+    valOrFunc => {
+      try {
+        const newState = typeof valOrFunc === 'function' ? (valOrFunc as Function)(state) : valOrFunc;
+        if (typeof newState === 'undefined') return;
+        let value: string;
+
+        if (options)
+          if (options.raw)
+            if (typeof newState === 'string') value = newState;
+            else value = JSON.stringify(newState);
+          else if (options.serializer) value = options.serializer(newState);
+          else value = JSON.stringify(newState);
+        else value = JSON.stringify(newState);
+
+        localStorage.setItem(key, value);
+        setState(deserializer(value));
+      } catch {
+        // If user is in private mode or has storage restriction
+        // localStorage can throw. Also JSON.stringify can throw.
+      }
+    },
+    [key, setState]
+  );
+
   const remove = useCallback(() => {
     try {
       localStorage.removeItem(key);
@@ -54,16 +81,7 @@ const useLocalStorage = <T>(
     }
   }, [key, setState]);
 
-  useEffect(() => {
-    if (isUndefined(state)) return;
-    try {
-      localStorage.setItem(key, serializer(state));
-    } catch {
-      // If user is in private mode or has storage restriction
-      // localStorage can throw. Also JSON.stringify can throw.
-    }
-  }, [state]);
-  return [state, setState, remove];
+  return [state, set, remove];
 };
 
 export default useLocalStorage;
diff --git a/tests/setupTests.ts b/tests/setupTests.ts
diff --git a/tests/useLocalStorage.test.ts b/tests/useLocalStorage.test.ts
@@ -1,95 +1,236 @@
+import useLocalStorage from '../src/useLocalStorage';
+import 'jest-localstorage-mock';
 import { renderHook, act } from '@testing-library/react-hooks';
-import { useLocalStorage } from '../src';
 
-const STRINGIFIED_VALUE = '{"a":"b"}';
-const JSONIFIED_VALUE = { a: 'b' };
+describe(useLocalStorage, () => {
+  afterEach(() => {
+    localStorage.clear();
+    jest.clearAllMocks();
+  });
 
-afterEach(() => {
-  localStorage.clear();
-  jest.clearAllMocks();
-});
+  it('retrieves an existing value from localStorage', () => {
+    localStorage.setItem('foo', '"bar"');
+    const { result } = renderHook(() => useLocalStorage('foo'));
+    const [state] = result.current;
+    expect(state).toEqual('bar');
+  });
 
-it('should return undefined if no initialValue provided and localStorage empty', () => {
-  const { result } = renderHook(() => useLocalStorage('some_key'));
+  it('should return initialValue if localStorage empty and set that to localStorage', () => {
+    const { result } = renderHook(() => useLocalStorage('foo', 'bar'));
+    const [state] = result.current;
+    expect(state).toEqual('bar');
+    expect(localStorage.__STORE__.foo).toEqual('"bar"');
+  });
 
-  expect(result.current[0]).toBeUndefined();
-});
+  it('prefers existing value over initial state', () => {
+    localStorage.setItem('foo', '"bar"');
+    const { result } = renderHook(() => useLocalStorage('foo', 'baz'));
+    const [state] = result.current;
+    expect(state).toEqual('bar');
+  });
 
-it('should set the value from existing localStorage key', () => {
-  const key = 'some_key';
-  localStorage.setItem(key, STRINGIFIED_VALUE);
+  it('does not clobber existing localStorage with initialState', () => {
+    localStorage.setItem('foo', '"bar"');
+    const { result } = renderHook(() => useLocalStorage('foo', 'buzz'));
+    result.current; // invoke current to make sure things are set
+    expect(localStorage.__STORE__.foo).toEqual('"bar"');
+  });
 
-  const { result } = renderHook(() => useLocalStorage(key));
+  it('correctly updates localStorage', () => {
+    const { result, rerender } = renderHook(() => useLocalStorage('foo', 'bar'));
 
-  expect(result.current[0]).toEqual(JSONIFIED_VALUE);
-});
+    const [, setFoo] = result.current;
+    act(() => setFoo('baz'));
+    rerender();
 
-it('should return initialValue if localStorage empty and set that to localStorage', () => {
-  const key = 'some_key';
-  const value = 'some_value';
+    expect(localStorage.__STORE__.foo).toEqual('"baz"');
+  });
 
-  const { result } = renderHook(() => useLocalStorage(key, value));
+  it('should return undefined if no initialValue provided and localStorage empty', () => {
+    const { result } = renderHook(() => useLocalStorage('some_key'));
 
-  expect(result.current[0]).toBe(value);
-  expect(localStorage.__STORE__[key]).toBe(`"${value}"`);
-});
+    expect(result.current[0]).toBeUndefined();
+  });
 
-it('should return the value from localStorage if exists even if initialValue provied', () => {
-  const key = 'some_key';
-  localStorage.setItem(key, STRINGIFIED_VALUE);
+  it('returns and allow setting null', () => {
+    localStorage.setItem('foo', 'null');
+    const { result, rerender } = renderHook(() => useLocalStorage('foo'));
 
-  const { result } = renderHook(() => useLocalStorage(key, 'random_value'));
+    const [foo1, setFoo] = result.current;
+    act(() => setFoo(null));
+    rerender();
 
-  expect(result.current[0]).toEqual(JSONIFIED_VALUE);
-});
+    const [foo2] = result.current;
+    expect(foo1).toEqual(null);
+    expect(foo2).toEqual(null);
+  });
 
-it('should properly update the localStorage on change', () => {
-  const key = 'some_key';
-  const updatedValue = { b: 'a' };
-  const expectedValue = '{"b":"a"}';
+  it('sets initialState if initialState is an object', () => {
+    renderHook(() => useLocalStorage('foo', { bar: true }));
+    expect(localStorage.__STORE__.foo).toEqual('{"bar":true}');
+  });
+
+  it('correctly and promptly returns a new value', () => {
+    const { result, rerender } = renderHook(() => useLocalStorage('foo', 'bar'));
 
-  const { result } = renderHook(() => useLocalStorage(key));
+    const [, setFoo] = result.current;
+    act(() => setFoo('baz'));
+    rerender();
 
-  act(() => {
-    result.current[1](updatedValue);
+    const [foo] = result.current;
+    expect(foo).toEqual('baz');
   });
 
-  expect(result.current[0]).toBe(updatedValue);
-  expect(localStorage.__STORE__[key]).toBe(expectedValue);
-});
+  /*
+  it('keeps multiple hooks accessing the same key in sync', () => {
+    localStorage.setItem('foo', 'bar');
+    const { result: r1, rerender: rerender1 } = renderHook(() => useLocalStorage('foo'));
+    const { result: r2, rerender: rerender2 } = renderHook(() => useLocalStorage('foo'));
 
-describe('Options with raw true', () => {
-  it('should set the value from existing localStorage key', () => {
-    const key = 'some_key';
-    localStorage.setItem(key, STRINGIFIED_VALUE);
+    const [, setFoo] = r1.current;
+    act(() => setFoo('potato'));
+    rerender1();
+    rerender2();
 
-    const { result } = renderHook(() => useLocalStorage(key, '', { raw: true }));
+    const [val1] = r1.current;
+    const [val2] = r2.current;
 
-    expect(result.current[0]).toEqual(STRINGIFIED_VALUE);
+    expect(val1).toEqual(val2);
+    expect(val1).toEqual('potato');
+    expect(val2).toEqual('potato');
   });
+  */
 
-  it('should return initialValue if localStorage empty and set that to localStorage', () => {
-    const key = 'some_key';
+  it('parses out objects from localStorage', () => {
+    localStorage.setItem('foo', JSON.stringify({ ok: true }));
+    const { result } = renderHook(() => useLocalStorage<{ ok: boolean }>('foo'));
+    const [foo] = result.current;
+    expect(foo!.ok).toEqual(true);
+  });
+
+  it('safely initializes objects to localStorage', () => {
+    const { result } = renderHook(() => useLocalStorage<{ ok: boolean }>('foo', { ok: true }));
+    const [foo] = result.current;
+    expect(foo!.ok).toEqual(true);
+  });
+
+  it('safely sets objects to localStorage', () => {
+    const { result, rerender } = renderHook(() => useLocalStorage<{ ok: any }>('foo', { ok: true }));
 
-    const { result } = renderHook(() => useLocalStorage(key, STRINGIFIED_VALUE, { raw: true }));
+    const [, setFoo] = result.current;
+    act(() => setFoo({ ok: 'bar' }));
+    rerender();
 
-    expect(result.current[0]).toBe(STRINGIFIED_VALUE);
-    expect(localStorage.__STORE__[key]).toBe(STRINGIFIED_VALUE);
+    const [foo] = result.current;
+    expect(foo!.ok).toEqual('bar');
   });
-});
 
-describe('Options with raw false and provided serializer/deserializer', () => {
-  const serializer = (_: string) => '321';
-  const deserializer = (_: string) => '123';
+  it('safely returns objects from updates', () => {
+    const { result, rerender } = renderHook(() => useLocalStorage<{ ok: any }>('foo', { ok: true }));
+
+    const [, setFoo] = result.current;
+    act(() => setFoo({ ok: 'bar' }));
+    rerender();
 
-  it('should return valid serialized value from existing localStorage key', () => {
-    const key = 'some_key';
-    localStorage.setItem(key, STRINGIFIED_VALUE);
+    const [foo] = result.current;
+    expect(foo).toBeInstanceOf(Object);
+    expect(foo!.ok).toEqual('bar');
+  });
 
-    const { result } = renderHook(() =>
-      useLocalStorage(key, STRINGIFIED_VALUE, { raw: false, serializer, deserializer })
+  it('sets localStorage from the function updater', () => {
+    const { result, rerender } = renderHook(() =>
+      useLocalStorage<{ foo: string; fizz?: string }>('foo', { foo: 'bar' })
     );
 
-    expect(result.current[0]).toBe('123');
+    const [, setFoo] = result.current;
+    act(() => setFoo(state => ({ ...state!, fizz: 'buzz' })));
+    rerender();
+
+    const [value] = result.current;
+    expect(value!.foo).toEqual('bar');
+    expect(value!.fizz).toEqual('buzz');
+  });
+
+  it('rejects nullish or undefined keys', () => {
+    const { result } = renderHook(() => useLocalStorage(null as any));
+    try {
+      result.current;
+      fail('hook should have thrown');
+    } catch (e) {
+      expect(String(e)).toMatch(/key may not be/i);
+    }
+  });
+
+  /* Enforces proper eslint react-hooks/rules-of-hooks usage */
+  describe('eslint react-hooks/rules-of-hooks', () => {
+    it('memoizes an object between rerenders', () => {
+      const { result, rerender } = renderHook(() => useLocalStorage('foo', { ok: true }));
+
+      result.current; // if localStorage isn't set then r1 and r2 will be different
+      rerender();
+      const [r2] = result.current;
+      rerender();
+      const [r3] = result.current;
+      expect(r2).toBe(r3);
+    });
+
+    it('memoizes an object immediately if localStorage is already set', () => {
+      localStorage.setItem('foo', JSON.stringify({ ok: true }));
+      const { result, rerender } = renderHook(() => useLocalStorage('foo', { ok: true }));
+
+      const [r1] = result.current; // if localStorage isn't set then r1 and r2 will be different
+      rerender();
+      const [r2] = result.current;
+      expect(r1).toBe(r2);
+    });
+
+    it('memoizes the setState function', () => {
+      localStorage.setItem('foo', JSON.stringify({ ok: true }));
+      const { result, rerender } = renderHook(() => useLocalStorage('foo', { ok: true }));
+      const [, s1] = result.current;
+      rerender();
+      const [, s2] = result.current;
+      expect(s1).toBe(s2);
+    });
+  });
+
+  describe('Options: raw', () => {
+    it('returns a string when localStorage is a stringified object', () => {
+      localStorage.setItem('foo', JSON.stringify({ fizz: 'buzz' }));
+      const { result } = renderHook(() => useLocalStorage('foo', null, { raw: true }));
+      const [foo] = result.current;
+      expect(typeof foo).toBe('string');
+    });
+
+    it('returns a string after an update', () => {
+      localStorage.setItem('foo', JSON.stringify({ fizz: 'buzz' }));
+      const { result, rerender } = renderHook(() => useLocalStorage('foo', null, { raw: true }));
+
+      const [, setFoo] = result.current;
+
+      act(() => setFoo({ fizz: 'bang' } as any));
+      rerender();
+
+      const [foo] = result.current;
+      expect(typeof foo).toBe('string');
+
+      expect(JSON.parse(foo!)).toBeInstanceOf(Object);
+
+      // expect(JSON.parse(foo!).fizz).toEqual('bang');
+    });
+
+    it('still forces setState to a string', () => {
+      localStorage.setItem('foo', JSON.stringify({ fizz: 'buzz' }));
+      const { result, rerender } = renderHook(() => useLocalStorage('foo', null, { raw: true }));
+
+      const [, setFoo] = result.current;
+
+      act(() => setFoo({ fizz: 'bang' } as any));
+      rerender();
+
+      const [value] = result.current;
+
+      expect(JSON.parse(value!).fizz).toEqual('bang');
+    });
   });
 });