From 688dd21a69038ae73869d4fa7400502b4aa7f80e 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 This pull request depends on pull requests #1021 #979 --- src/useStorage.ts | 46 +++++++++++++++++++++++------ stories/useLocalStorage.story.tsx | 29 +++++++++++------- stories/useSessionStorage.story.tsx | 23 +++++++++++---- tests/useLocalStorage.test.ts | 19 ++++++++++++ tests/useSessionStorage.test.ts | 18 +++++++++++ tests/useStorage.test.ts | 36 ++++++++++++++++++++++ 6 files changed, 146 insertions(+), 25 deletions(-) create mode 100644 tests/useStorage.test.ts diff --git a/src/useStorage.ts b/src/useStorage.ts index 4c330a3510..9bcd9818ed 100644 --- a/src/useStorage.ts +++ b/src/useStorage.ts @@ -1,6 +1,20 @@ /* eslint-disable */ -import { useState, useCallback, Dispatch, SetStateAction, useMemo } from 'react'; +import { useCallback, Dispatch, SetStateAction, useMemo } from 'react'; import { isClient } from './util'; +import { createGlobalState, useStateReturnType } from './createGlobalState' + +type DispatchAction = Dispatch>; +type localStateHook = (initialState?: SetStateAction) => useStateReturnType; +type storageKeyHooks = { + [storageType: string]: { + [key: string]: localStateHook + } +}; +let useStorageKeyHook: storageKeyHooks = {} +// This is useful for testing +export const resetStorageState = () => { + useStorageKeyHook = {} +} export type parserOptions = | { @@ -19,8 +33,9 @@ const useStorage = ( key: string, initialValue?: T, options?: parserOptions -): [T | undefined, Dispatch>, () => void] => { - if (!isClient) { +): [T | undefined, DispatchAction, () => void] => { + debugger; +if (!isClient) { return [initialValue as T, noop, noop]; } if (!key) { @@ -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,18 +62,30 @@ 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 set: Dispatch> = useCallback( + const set: DispatchAction = useCallback( valOrFunc => { try { - const newState = typeof valOrFunc === 'function' ? (valOrFunc as Function)(state) : valOrFunc; + const newState = valOrFunc instanceof Function + ? valOrFunc(state) + : valOrFunc; if (typeof newState === 'undefined') return; let value: string; @@ -72,9 +99,10 @@ 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] 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 da01dc8698..4936c1c82e 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', () => { @@ -17,6 +19,7 @@ describe(useLocalStorage, () => { }); it('should return initialValue if localStorage empty and set that to localStorage', () => { + debugger; const { result } = renderHook(() => useLocalStorage('foo', 'bar')); const [state] = result.current; expect(state).toEqual('bar'); @@ -234,4 +237,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 === 'bar'); + expect(state2 === 'bar'); + act(() => { + setState1('baz'); + }); + [ state1, setState1 ] = result1.current; + [ state2 ] = result2.current; + expect(state1 === 'baz'); + expect(state2 === 'baz'); + }); }); diff --git a/tests/useSessionStorage.test.ts b/tests/useSessionStorage.test.ts index ff12423bb2..c2e40c979c 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 === 'bar'); + expect(state2 === 'bar'); + act(() => { + setState1('baz'); + }); + [ state1, setState1 ] = result1.current; + [ state2 ] = result2.current; + expect(state1 === 'baz'); + expect(state2 === 'baz'); + }); }); diff --git a/tests/useStorage.test.ts b/tests/useStorage.test.ts new file mode 100644 index 0000000000..1276fa119a --- /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 === 'baz'); + expect(state2 === 'bar'); + act(() => { + setState1('baz-new'); + }); + [ state1, setState1 ] = result1.current; + [ state2, setState2 ] = result2.current; + expect(state1 === 'baz-new'); + expect(state2 === 'bar'); + act(() => { + setState2('bar-new'); + }); + [ state1, setState1 ] = result1.current; + [ state2, setState2 ] = result2.current; + expect(state1 === 'baz-new'); + expect(state2 === 'bar-new'); + }); +});