From 616f1c352c0ba10ec27a4c5299df280c0b65f675 Mon Sep 17 00:00:00 2001 From: Nathan Spaeth Date: Thu, 5 Mar 2020 01:08:14 -0600 Subject: [PATCH] fix(#785): update useStorage to use createGlobalState Fixes #785 - useLocalStorage not updating if many components watch the same key Previously pr #786 addressed this issue by ensuring that other components watching the key would see new updates, however, those updates would not be rendered until something else triggered a re-render. This pr resolves that issue. This pull request depends on pull requests #1021 #979 --- src/useStorage.ts | 66 +++++++++++++++++++++-------- stories/useLocalStorage.story.tsx | 29 ++++++++----- stories/useSessionStorage.story.tsx | 23 +++++++--- tests/useLocalStorage.test.ts | 18 ++++++++ tests/useSessionStorage.test.ts | 18 ++++++++ tests/useStorage.test.ts | 36 ++++++++++++++++ 6 files changed, 156 insertions(+), 34 deletions(-) create mode 100644 tests/useStorage.test.ts diff --git a/src/useStorage.ts b/src/useStorage.ts index 4c330a3510..41748f0509 100644 --- a/src/useStorage.ts +++ b/src/useStorage.ts @@ -1,6 +1,21 @@ /* eslint-disable */ -import { useState, useCallback, Dispatch, SetStateAction, useMemo } from 'react'; +import { useCallback, Dispatch, useMemo } from 'react'; +import { InitialHookState, HookState, resolveHookState, } from './util/resolveHookState'; import { isClient } from './util'; +import { createGlobalState, GlobalStateHookReturn } from './createGlobalState'; + +type DispatchAction = Dispatch>; +type localStateHook = (initialState?: InitialHookState) => GlobalStateHookReturn; +type storageKeyHooks = { + [storageType: string]: { + [key: string]: localStateHook + } +}; +let useStorageKeyHook: storageKeyHooks = {} +// This is useful for testing +export const resetStorageState = () => { + useStorageKeyHook = {} +} export type parserOptions = | { @@ -19,7 +34,7 @@ const useStorage = ( key: string, initialValue?: T, options?: parserOptions -): [T | undefined, Dispatch>, () => void] => { +): [T | undefined, DispatchAction, () => void] => { if (!isClient) { return [initialValue as T, noop, noop]; } @@ -36,7 +51,7 @@ const useStorage = ( const deserializer = options ? (options.raw ? value => value : options.deserializer) : JSON.parse; - const [state, setState] = useState(() => { + const setInitialState = () => { try { const serializer = options ? (options.raw ? String : options.serializer) : JSON.stringify; @@ -47,19 +62,41 @@ const useStorage = ( initialValue && storage.setItem(key, serializer(initialValue)); return initialValue; } - } catch { + } catch (error) { // If user is in private mode or has storage restriction // storage can throw. JSON.parse and JSON.stringify // can throw, too. + console.error(error) return initialValue; } - }); + }; + + if (!useStorageKeyHook[storageType]) { + useStorageKeyHook[storageType] = {} + } + if (!useStorageKeyHook[storageType][key]) { + useStorageKeyHook[storageType][key] = createGlobalState(undefined) + } + const useStorageState: localStateHook = useStorageKeyHook[storageType][key] + const [ state, setState ] = useStorageState(setInitialState) + + const removeKey = () => { + try { + storage.removeItem(key); + setState(undefined); + } catch(error) { + // If user is in private mode or has storage restriction + // storage can throw. + console.error(error); + } + } - const set: Dispatch> = useCallback( + const set: DispatchAction = useCallback( valOrFunc => { try { - const newState = typeof valOrFunc === 'function' ? (valOrFunc as Function)(state) : valOrFunc; - if (typeof newState === 'undefined') return; + const newState = resolveHookState(valOrFunc, state) + // if (typeof newState === 'undefined') return; + if (typeof newState === 'undefined') return removeKey() let value: string; if (options) @@ -72,23 +109,16 @@ const useStorage = ( storage.setItem(key, value); setState(deserializer(value)); - } catch { + } catch (error) { // If user is in private mode or has storage restriction // storage can throw. Also JSON.stringify can throw. + console.error(error); } }, [key, setState] ); - const remove = useCallback(() => { - try { - storage.removeItem(key); - setState(undefined); - } catch { - // If user is in private mode or has storage restriction - // storage can throw. - } - }, [key, setState]); + const remove = useCallback(removeKey, [key, setState]); return [state, set, remove]; }; diff --git a/stories/useLocalStorage.story.tsx b/stories/useLocalStorage.story.tsx index f246c9607f..dd696b366d 100644 --- a/stories/useLocalStorage.story.tsx +++ b/stories/useLocalStorage.story.tsx @@ -3,21 +3,28 @@ import * as React from 'react'; import { useLocalStorage } from '../src'; import ShowDocs from './util/ShowDocs'; +const StorageKey = ({ storageKey }) => { + const [value, setValue, remove] = useLocalStorage(storageKey); + return ( +
+
(Storage key: { storageKey }) Value: {value}
+ + + +
+
+ ); +} + const Demo = () => { - const [value, setValue] = useLocalStorage('hello-key', 'foo'); - const [removableValue, setRemovableValue, remove] = useLocalStorage('removeable-key'); + useLocalStorage('hello-key', 'initialValue'); + useLocalStorage('no-initial-value'); return (
-
Value: {value}
- - -
-
-
Removable Value: {removableValue}
- - - + + +
); }; diff --git a/stories/useSessionStorage.story.tsx b/stories/useSessionStorage.story.tsx index 28fdf9a4cb..c1001b1bb5 100644 --- a/stories/useSessionStorage.story.tsx +++ b/stories/useSessionStorage.story.tsx @@ -3,15 +3,28 @@ import * as React from 'react'; import { useSessionStorage } from '../src'; import ShowDocs from './util/ShowDocs'; +const StorageKey = ({ storageKey }) => { + const [value, setValue, remove] = useSessionStorage(storageKey); + return ( +
+
(Storage key: { storageKey }) Value: {value}
+ + + +
+
+ ); +} + const Demo = () => { - const [value, setValue, remove] = useSessionStorage('hello-key', 'foo'); + useSessionStorage('hello-key', 'initialValue'); + useSessionStorage('no-initial-value'); return (
-
Value: {value}
- - - + + +
); }; diff --git a/tests/useLocalStorage.test.ts b/tests/useLocalStorage.test.ts index 40387e8755..39c701050c 100644 --- a/tests/useLocalStorage.test.ts +++ b/tests/useLocalStorage.test.ts @@ -1,5 +1,6 @@ /* eslint-disable */ import useLocalStorage from '../src/useLocalStorage'; +import { resetStorageState } from '../src/useStorage'; import 'jest-localstorage-mock'; import { renderHook, act } from '@testing-library/react-hooks'; @@ -7,6 +8,7 @@ describe(useLocalStorage, () => { afterEach(() => { localStorage.clear(); jest.clearAllMocks(); + resetStorageState(); }); it('retrieves an existing value from localStorage', () => { @@ -233,4 +235,20 @@ describe(useLocalStorage, () => { expect(JSON.parse(value!).fizz).toEqual('bang'); }); }); + + it('both components should be updated', () => { + const { result: result1 } = renderHook(() => useLocalStorage('foo', 'bar')); + const { result: result2 } = renderHook(() => useLocalStorage('foo')); + let [ state1, setState1 ] = result1.current; + let [ state2 ] = result2.current; + expect(state1).toBe('bar'); + expect(state2).toBe('bar'); + act(() => { + setState1('baz'); + }); + [ state1, setState1 ] = result1.current; + [ state2 ] = result2.current; + expect(state1).toBe('baz'); + expect(state2).toBe('baz'); + }); }); diff --git a/tests/useSessionStorage.test.ts b/tests/useSessionStorage.test.ts index ff12423bb2..ed97784eb9 100644 --- a/tests/useSessionStorage.test.ts +++ b/tests/useSessionStorage.test.ts @@ -1,5 +1,6 @@ /* eslint-disable */ import useSessionStorage from '../src/useSessionStorage'; +import { resetStorageState } from '../src/useStorage'; import 'jest-localstorage-mock'; import { renderHook, act } from '@testing-library/react-hooks'; @@ -7,6 +8,7 @@ describe(useSessionStorage, () => { afterEach(() => { sessionStorage.clear(); jest.clearAllMocks(); + resetStorageState(); }); it('retrieves an existing value from sessionStorage', () => { @@ -234,4 +236,20 @@ describe(useSessionStorage, () => { expect(JSON.parse(value!).fizz).toEqual('bang'); }); }); + + it('both components should be updated', () => { + const { result: result1 } = renderHook(() => useSessionStorage('foo', 'bar')); + const { result: result2 } = renderHook(() => useSessionStorage('foo')); + let [ state1, setState1 ] = result1.current; + let [ state2 ] = result2.current; + expect(state1).toBe('bar'); + expect(state2).toBe('bar'); + act(() => { + setState1('baz'); + }); + [ state1, setState1 ] = result1.current; + [ state2 ] = result2.current; + expect(state1).toBe('baz'); + expect(state2).toBe('baz'); + }); }); diff --git a/tests/useStorage.test.ts b/tests/useStorage.test.ts new file mode 100644 index 0000000000..304194fd82 --- /dev/null +++ b/tests/useStorage.test.ts @@ -0,0 +1,36 @@ +/* eslint-disable */ +import useStorage, { resetStorageState } from '../src/useStorage'; +import 'jest-localstorage-mock'; +import { renderHook, act } from '@testing-library/react-hooks'; + +describe(useStorage, () => { + afterEach(() => { + sessionStorage.clear(); + localStorage.clear(); + jest.clearAllMocks(); + resetStorageState(); + }); + + it('localStorage keys should not conflict with sessionStorage keys', () => { + const { result: result1 } = renderHook(() => useStorage('localStorage', 'foo', 'baz', { raw: true })); + const { result: result2 } = renderHook(() => useStorage('sessionStorage', 'foo', 'bar', { raw: true })); + let [ state1, setState1 ] = result1.current; + let [ state2, setState2 ] = result2.current; + expect(state1).toBe('baz'); + expect(state2).toBe('bar'); + act(() => { + setState1('baz-new'); + }); + [ state1, setState1 ] = result1.current; + [ state2, setState2 ] = result2.current; + expect(state1).toBe('baz-new'); + expect(state2).toBe('bar'); + act(() => { + setState2('bar-new'); + }); + [ state1, setState1 ] = result1.current; + [ state2, setState2 ] = result2.current; + expect(state1).toBe('baz-new'); + expect(state2).toBe('bar-new'); + }); +});