react-use icon indicating copy to clipboard operation
react-use copied to clipboard

Fix: fix useLocalStorage func setState to have correct state

Open nowells opened this issue 3 years ago • 3 comments

Description

Right now the useLocalStorage hook does not properly add state as a useCallback dependency, leading to bugs where you pass a function as the argument to setState

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue)

Checklist

  • [x] Read the Contributing Guide
  • [x] Perform a code self-review
  • [x] Comment the code, particularly in hard-to-understand areas
  • [x] Add documentation
  • [x] Add hook's story at Storybook
  • [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).

Your eslint fails due to configuration changes in prettier@8 which I am guessing happened a while ago but is not tested against.

nowells avatar Jun 22 '21 14:06 nowells

Spent about an hour trying to setState using callback function, ended up creating my own hook for now

import { useEffect, useState } from 'react';

export default function useLocalstorage<T>(
  key: string,
  initialVal: T,
): [T, React.Dispatch<React.SetStateAction<T>>] {
  const [state, setState] = useState<T>(initialVal);

  useEffect(() => {
    const localStorageValue = window.localStorage.getItem(key);
    if (localStorageValue !== null) {
      setState(JSON.parse(localStorageValue));
    }
  }, [key]);

  useEffect(() => {
    window.localStorage.setItem(key, JSON.stringify(state));
  }, [state, key]);

  return [state, setState];
}

BhavyaCodes avatar Sep 23 '21 19:09 BhavyaCodes

This is not enough in every case. If something like set(s => s + 1) is called multiple times, before a rerender, it would produce a wrong result, as the old state is used twice.

ribx avatar Apr 30 '22 10:04 ribx

I believe that instead of using useState the state should be kept in a ref thus exposing access to the latest value when needed. Then useUpdate can be used to trigger renders when the state changes.

edzis avatar Jul 18 '22 15:07 edzis