tbleckert / react-select-search

⚡️ Lightweight select component for React
https://react-select-search.com
MIT License
674 stars 149 forks source link

Can't state update on unmounted component warning when SelectSearch with getOptions flashes #225

Open tada-s opened 2 years ago

tada-s commented 2 years ago

In this scenario, the SelectSearch has a getOptions property set. When this component flashes (i.e., it is unmounted and mounted rapidly), the following warning raises on the browser:

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.

I found in useFetch.js Line 22 that setFetching() was the state update that causes this warning.

Minimal working example

import React, { useState } from "react";
import SelectSearch from "react-select-search";

export default function App() {
  const [visible, setVisible] = useState(true);

  React.useEffect(() => {
    setVisible(false);
    setTimeout(() => {
      setVisible(true);
    }, 0);
  }, []);

  const getOptions = (q) => [];

  return visible ? (
    <SelectSearch options={[]} getOptions={getOptions} />
  ) : (
    <></>
  );
}

Workaround Add a new state mounted in useFetch(), add a useEffect managing the state mounted and fetch the option if is mounted.

export default function useFetch(...) {
    const [mounted, setMounted] = useState(false); // Add a new state
    ...
    ...

    const fetch = useMemo(() => {
        const filter = filterOptions || ((op) => () => op);

        if (!getOptions) {
            return (s) => setOptions(flattenOptions(filter(defaultOptions)(s)));
        }

        if (!mounted) { // If is not mounted, return immediately
            return (s) => [];
        }

        return debounce((s) => {
            const optionsReq = getOptions(s, defaultOptions);

            setFetching(true);

            Promise.resolve(optionsReq)
                .then((newOptions) => {
                    setOptions(flattenOptions(filter(newOptions)(s)));
                })
                .finally(() => setFetching(false));
        }, debounceTime);
    }, [filterOptions, defaultOptions, getOptions, debounceTime]);

    ...
    ...

    useEffect(() => { // Track the mounted state
        setMounted(true);
        return () => setMounted(false);
    ), []);

    return { options, setOptions, fetching };
}

Do you think this workaround is good enough for a pull request?

tbleckert commented 2 years ago

Thanks for the report @tada-s . It's a good workaround but would also need to exist inside the debounced promise. A more robust solution would be to move the setOptions call to a function and have your mount check there.