Or Assayag
Or Assayag

Reputation: 6336

React memory leak warning in useEffect

The following code gives me leak memory warning:

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.

const prepareMetaData = async () => {
    const languagesJson = localStorage.getItem('feedsLanguages');
    const platformModelsJson = localStorage.getItem('feedsPlatformModels');

    let languagesArr;
    let platformsArr;
    if (!ValidationUtils.isExists(languagesJson) || !ValidationUtils.isExists(platformModelsJson)) {
      const res = await fetch(buildApiUrl('feeds/meta-data', true, false), buildFetchRequest('GET'));
      await handleFailedFetchRequests(res);
      const metaData = await res.json();
      languagesArr = metaData.languages;
      platformsArr = metaData.platformModels;
      localStorage.setItem('feedsLanguages', JSON.stringify(metaData.languages));
      localStorage.setItem('feedsPlatformModels', JSON.stringify(metaData.platformModels));
    } else {
      languagesArr = JSON.parse(languagesJson);
      platformsArr = JSON.parse(platformModelsJson);
    }

    const langObj = TextUtils.setLanguages(languagesArr);
    setLanguagesDict(langObj);
    setPlatformModels(platformsArr);
  };

  const prepareData = async () => {
      prepareMetaData();
  };

  useEffect(() => {
    prepareData();
  }, []);

So I tried the following:

let isCanceled = false;
const prepareMetaData = async () => {
    const languagesJson = localStorage.getItem('feedsLanguages');
    const platformModelsJson = localStorage.getItem('feedsPlatformModels');

    let languagesArr;
    let platformsArr;
    if (!ValidationUtils.isExists(languagesJson) || !ValidationUtils.isExists(platformModelsJson)) {
      const res = await fetch(buildApiUrl('feeds/meta-data', true, false), buildFetchRequest('GET'));
      await handleFailedFetchRequests(res);
      const metaData = await res.json();
      languagesArr = metaData.languages;
      platformsArr = metaData.platformModels;
      localStorage.setItem('feedsLanguages', JSON.stringify(metaData.languages));
      localStorage.setItem('feedsPlatformModels', JSON.stringify(metaData.platformModels));
    } else {
      languagesArr = JSON.parse(languagesJson);
      platformsArr = JSON.parse(platformModelsJson);
    }

    const langObj = TextUtils.setLanguages(languagesArr);
    if (!isCanceled) {
       setLanguagesDict(langObj);
       setPlatformModels(platformsArr);
    }
  };

  const prepareData = async () => {
      prepareMetaData();
  };

  useEffect(() => {
    prepareData();
    return () => {
      isCanceled = true;
    }
  }, []);

But it's still not working. Any working solution?
Thanks.

Upvotes: 0

Views: 332

Answers (2)

In async functions, you can't be sure that the two setState calls run before the component could unmount.

setLanguagesDict(langObj);
// other stuff can happen here
setPlatformModels(platformsArr);

To solve this, with your solution, you would need to check isCancel before each call separately.

if (!isCanceled) {
    setLanguagesDict(langObj);
}
if (!isCanceled) {
    setPlatformModels(platformsArr);
}

To confirm / replicate the issue (and prove that the solution works) you can check this REPL.it (specifically the App.js) file.

To be clear - this is a very messy solution to the problem. I would consider avoiding the re-render, either by using refs or a reducer pattern (depending on the usage context obviously).

In short, I took the create react app template and created a very silly App.js that unmounts after the first setState, returning after an effect runs. Run it with the console open and you will see the message occur. Uncomment the in-between test to see it fixed.

import React from 'react';
import logo from './logo.svg';
import './App.css';

let internalCounter = 0;

let toggledIsActive = false;

const TogglingComponent = ({
    initialCounter,
    setCounter,
}) => {
    const [counterToShow, setInternalCounter] = React.useState(initialCounter);
    React.useEffect(() => {
        toggledIsActive = true;
        const waitAndUpdate = async () => {
            await new Promise(resolve => {
                setTimeout(() => resolve(), 1500);
            });
            internalCounter++;
            if (toggledIsActive) {
                setCounter(internalCounter);
            // Uncomment to fix state update
            // }
            // if (toggledIsActive) {
                setInternalCounter(internalCounter);
            }
        }

        const interval = setInterval(waitAndUpdate, 1500);
        return () => {
            toggledIsActive = false;
            clearInterval(interval);
        };
    });
    return <a
        className="App-link"
        href="https://reactjs.org"
        target="_blank"
        rel="noopener noreferrer"
    >
        Learn React {counterToShow}
    </a>;
}

function App() {
  const [counter, setCounter] = React.useState(0);

  const [lastCounter, setLastCounter] = React.useState(counter);
  React.useEffect(() => {
      setLastCounter(counter);
  }, [counter])

  return (
    <div className="App">
      <header className="App-header">
        <img src={logo} className="App-logo" alt="logo" />
        <p>
          Edit <code>src/App.js</code>!
        </p>
        {counter === lastCounter ? <TogglingComponent
        initialCounter={lastCounter}
        setCounter={setCounter}
        /> : null}
      </header>
    </div>
  );
}

Upvotes: 1

Or Assayag
Or Assayag

Reputation: 6336

I solved it by merging the 2 setStates into one, like the following:

setAdditionalProps({
  languages: langObj,
  platforms: platformsArr,
});

It solved my issue. Thanks all.

Upvotes: 1

Related Questions