Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use local storage #786

Merged
merged 15 commits into from
Feb 3, 2020
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
"gh-pages": "2.1.1",
"husky": "3.1.0",
"jest": "24.9.0",
"jest-localstorage-mock": "^2.4.0",
"keyboardjs": "2.5.1",
"lint-staged": "9.4.3",
"markdown-loader": "5.1.0",
Expand Down
66 changes: 39 additions & 27 deletions src/useLocalStorage.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,52 @@
import { useEffect, useState } from 'react';
import { isClient } from './util';

type Dispatch<A> = (value: A) => void;
type SetStateAction<S> = S | ((prevState: S) => S);
import { useMemo, useCallback, useEffect, Dispatch, SetStateAction } from 'react';

const useLocalStorage = <T>(key: string, initialValue?: T, raw?: boolean): [T, Dispatch<SetStateAction<T>>] => {
if (!isClient) {
if (!isClient || !localStorage) {
return [initialValue as T, () => {}];
}
if (!key && (key as any) !== 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be a boolean false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can protect against that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what I was asking here looking back.

throw new Error('useLocalStorage key may not be nullish or undefined');
}

const [state, setState] = useState<T>(() => {
try {
const localStorageValue = localStorage.getItem(key);
if (typeof localStorageValue !== 'string') {
localStorage.setItem(key, raw ? String(initialValue) : JSON.stringify(initialValue));
return initialValue;
} else {
return raw ? localStorageValue : JSON.parse(localStorageValue || 'null');
}
} catch {
// If user is in private mode or has storage restriction
// localStorage can throw. JSON.parse and JSON.stringify
// can throw, too.
return initialValue;
}
});
let localStorageValue: string | null = null;
try {
localStorageValue = localStorage.getItem(key);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this expensive to be reading local storage every render?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. If you're trying to hit 60fps that gives you 16ms between rerenders for all your code to run.

From various benchmarks I'm seeing, a single read takes 0.0004-0.0010ms. Completely negligible.

If reading localStorage is someone's bottleneck, they have other issues they need to figure out.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah interesting. I've seen multiple tutorials making a hook like this that cite reading local storage as being synchronous and expensive as the reason for wanting to make a hook. I assumed that was based on reality but it must have just been used as a contrived example.

I started on a PR that used an object store to keep all the hooks in sync while caching the data but if it's not actually an issue that is over-optimization

} catch {
// If user is in private mode or has storage restriction
// localStorage can throw.
}

useEffect(() => {
const state: T = useMemo(() => {
try {
const serializedState = raw ? String(state) : JSON.stringify(state);
localStorage.setItem(key, serializedState);
/* If key hasn't been set yet */
if (localStorageValue === null) return initialValue as T;
return raw ? localStorageValue : JSON.parse(localStorageValue);
} catch {
// If user is in private mode or has storage restriction
// localStorage can throw. Also JSON.stringify can throw.
/* JSON.parse and JSON.stringify can throw. */
return localStorageValue === null ? initialValue : localStorageValue;
}
}, [state]);
}, [key, localStorageValue]);

const setState: Dispatch<SetStateAction<T>> = useCallback(
(valOrFunc: SetStateAction<T>): void => {
try {
let newState = typeof valOrFunc === 'function' ? (valOrFunc as Function)(state) : valOrFunc;
newState = typeof newState === 'string' ? newState : JSON.stringify(newState);
localStorage.setItem(key, newState);
} catch {
/**
* If user is in private mode or has storage restriction
* localStorage can throw. Also JSON.stringify can throw.
*/
}
},
[state, raw]
);

useEffect((): void => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could probably swap this out for useEffectOnce

if (localStorageValue === null && initialValue) setState(initialValue);
}, [localStorageValue, setState]);

return [state, setState];
};
Expand Down
211 changes: 211 additions & 0 deletions tests/useLocalStorage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
import useLocalStorage from '../src/useLocalStorage';
import 'jest-localstorage-mock';
import { renderHook, act } from '@testing-library/react-hooks';

describe(useLocalStorage, () => {
afterEach(() => localStorage.clear());
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('sets initial state', () => {
const { result } = renderHook(() => useLocalStorage('foo', 'bar'));
const [state] = result.current;
expect(state).toEqual('bar');
expect(localStorage.__STORE__.foo).toEqual('bar');
});
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('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');
});
it('correctly updates localStorage', () => {
const { result, rerender } = renderHook(() => useLocalStorage('foo', 'bar'));

const [, setFoo] = result.current;
act(() => setFoo('baz'));
rerender();

expect(localStorage.__STORE__.foo).toEqual('baz');
});
it('returns and allow setting null', () => {
localStorage.setItem('foo', 'null');
const { result, rerender } = renderHook(() => useLocalStorage('foo'));

const [foo1, setFoo] = result.current;
act(() => setFoo(null));
rerender();

const [foo2] = result.current;
expect(foo1).toEqual(null);
expect(foo2).toEqual(null);
});
it('correctly and promptly returns a new value', () => {
const { result, rerender } = renderHook(() => useLocalStorage('foo', 'bar'));

const [, setFoo] = result.current;
act(() => setFoo('baz'));
rerender();

const [foo] = result.current;
expect(foo).toEqual('baz');
});
it('should not double-JSON-stringify stringy values', () => {
const { result, rerender } = renderHook(() => useLocalStorage('foo', 'bar'));

const [, setFoo] = result.current;
act(() => setFoo(JSON.stringify('baz')));
rerender();

const [foo] = result.current;
expect(foo).not.toMatch(/\\/i); // should not contain extra escapes
expect(foo).toBe('baz');
});
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'));

const [, setFoo] = r1.current;
act(() => setFoo('potato'));
rerender1();
rerender2();

const [val1] = r1.current;
const [val2] = r2.current;

expect(val1).toEqual(val2);
expect(val1).toEqual('potato');
expect(val2).toEqual('potato');
});
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 [, setFoo] = result.current;
act(() => setFoo({ ok: 'bar' }));
rerender();

const [foo] = result.current;
expect(foo.ok).toEqual('bar');
});
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();

const [foo] = result.current;
expect(foo).toBeInstanceOf(Object);
expect(foo.ok).toEqual('bar');
});
it('sets localStorage from the function updater', () => {
const { result, rerender } = renderHook(() =>
useLocalStorage<{ foo: string; fizz?: string }>('foo', { foo: 'bar' })
);

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);
}
});
describe('raw setting', () => {
it('returns a string when localStorage is a stringified object', () => {
localStorage.setItem('foo', JSON.stringify({ fizz: 'buzz' }));
const { result } = renderHook(() => useLocalStorage('foo', null, 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, true));

const [, setFoo] = result.current;
// @ts-ignore
act(() => setFoo({ fizz: 'bang' }));
rerender();

const [foo] = result.current;
expect(typeof foo).toBe('string');
// @ts-ignore
expect(JSON.parse(foo)).toBeInstanceOf(Object);
// @ts-ignore
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, true));

const [, setFoo] = result.current;
// @ts-ignore
act(() => setFoo({ fizz: 'bang' }));
rerender();

const [value] = result.current;
// @ts-ignore
expect(JSON.parse(value).fizz).toEqual('bang');
});
});
/* 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);
});
});
});
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7384,6 +7384,11 @@ jest-leak-detector@^24.9.0:
jest-get-type "^24.9.0"
pretty-format "^24.9.0"

jest-localstorage-mock@^2.4.0:
version "2.4.0"
resolved "https://registry.yarnpkg.com/jest-localstorage-mock/-/jest-localstorage-mock-2.4.0.tgz#c6073810735dd3af74020ea6c3885ec1cc6d0d13"
integrity sha512-/mC1JxnMeuIlAaQBsDMilskC/x/BicsQ/BXQxEOw+5b1aGZkkOAqAF3nu8yq449CpzGtp5jJ5wCmDNxLgA2m6A==

jest-matcher-utils@^24.9.0:
version "24.9.0"
resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-24.9.0.tgz#f5b3661d5e628dffe6dd65251dfdae0e87c3a073"
Expand Down