streamich / react-use

React Hooks — 👍
http://streamich.github.io/react-use
The Unlicense
41.98k stars 3.16k forks source link

Remove potentially unnecessary use of useEffect in usePrevious #2605

Open RickHuizing01 opened 1 week ago

RickHuizing01 commented 1 week ago

Is your feature request related to a problem? Please describe.

The current implementation of the usePrevious hook relies on useEffect to store the previous state. While this approach works, I believe that the use of useEffect might not be necessary. The usage of useEffect could be considered an anti-pattern according to the official React documentation (as described in You Might Not Need an Effect) if I interpret it correctly.

Describe the solution you'd like

Initially, I considered removing the useEffect altogether, resulting in the following implementation:

import { useRef } from "react";

export const usePrevious = <T>(state: T): T | undefined => {
    const ref = useRef<T>();

    const previousValue = ref.current;
    ref.current = state;

    return previousValue;
};

While this solution works, a colleague pointed out that reading or writing to a ref during rendering is also considered an anti-pattern, as highlighted in the official React documentation here: https://react.dev/reference/react/useRef (see the pitfall section). Consequently, I created an alternative version that avoids both useRef and useEffect:

import { useState } from "react";

export const usePrevious = <T>(state: T): T | undefined => {
    const [current, setCurrent] = useState<T>(state);
    const [previous, setPrevious] = useState<T>();

    if (JSON.stringify(current) !== JSON.stringify(state)) {
        setCurrent(state);
        setPrevious(current);
    }

    return previous;
};

However, there is a significant pitfall with this implementation: the useState hook maintaining the current value essentially duplicates the state of the component using this hook. Based of that, the second implementation would make more sense if it was the useStateHistory hook as it naturally handles the "main" state.

I am still exploring alternative approaches but wanted to share my progress so far.