Repository: streamich/react-use
Pull request #786 (2019-11-20 08:52:18)

Fix use local storage
Addresses #785 

# Description  
Basically a total rewrite of `useLocalStorage` to ensure updates to the keyed value are picked up by all components watching for it.

## Type of change

<!-- Check all relevant options. -->
- [x] Bug fix _(non-breaking change which fixes an issue)_
- [ ] New feature _(non-breaking change which adds functionality)_
- [ ] **Breaking change** _(fix or feature that would cause existing functionality to not work as before)_

I don't _believe_ this will cause any breaking changes. But I did end up tearing out almost all the old code and fundamentally changed how this hook works. 🤷‍♂ 

# Checklist
- [x] Read the [Contributing Guide](https://github.com/streamich/react-use/blob/master/CONTRIBUTING.md)
  - [ ] better commit message
  - [ ] squash?
- [x] Perform a code self-review
- [x] Comment the code, particularly in hard-to-understand areas
- [x] Add documentation 
  - no changes
- [x] Add hook's story at Storybook
  - no changes
- [x] Cover changes with tests
- [x] Ensure the test suite passes (`yarn test`)
- [x] Provide 100% tests coverage
- [x] Make sure code lints (`yarn lint`). Fix it with `yarn lint:fix` in case of failure.
- [x] Make sure types are fine (`yarn lint:types`).
  - Could use more input for proper types, like `raw=true implies value=string`




Patch:
diff --git a/package.json b/package.json
index 977a9818..d4d196dc 100644
--- 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
index 65d38ebd..d843417c 100644
--- 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;


Test patch:
diff --git a/tests/setupTests.ts b/tests/setupTests.ts
deleted file mode 100644
index 1c787210..00000000
--- a/tests/setupTests.ts
+++ /dev/null
@@ -1,1 +0,0 @@
-import 'jest-localstorage-mock';
diff --git a/tests/useLocalStorage.test.ts b/tests/useLocalStorage.test.ts
index 95340c54..696aeb8c 100644
--- 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');
+    });
   });
 });