DEV Community

An elegant solution for memory leaks in React

Nans Dumortier on July 30, 2020

Cover image from Valentin Petkov from Unsplash An elegant solution for memory leaks in React 🔴 UPDATE This "solution" d...

Read full post

Steve Ruiz • Jul 31 '20

I agree with the other replies (that this is just hiding the warning) however I’d call it a failure of React’s error messaging. A “memory leak” is too ambiguous, given React’s user group, so better to provide some common causes and solutions.

In this case, you’re making a request that will effect the component somehow; so you don’t want to handle the response if it comes after the component has unmounted. Rather than waiting for the request to complete and then checking whether the component is still mounted, you should instead cancel the request when the component unmounts.

The request should be made in a way that allows it to be imperatively cancelled.
The useEffect should return a function that cancels the request.

Nans Dumortier • Jul 31 '20

Yeah, thanks for your answer, I just went through some test, and must admit that this hook doesn't really solve the leak problem.
Here is a screenshot of the evidence :

So you, Aleksandr and Xavier are right, thank you guys for pointing out!

I must say that I was mistaken when reading through this - kind of famous - lib's code. Being downloaded 17000+ times a week, I thought it would use good practices.

I'm going to update the documentation and the article to add a disclaimer about that, and in the meantime I will try to find another elegant solution.

Nans Dumortier • Jul 31 '20 • Edited

To my surprise, we are getting the same kind of graph with an abort controller (I simply took the example that I posted in this article.
It seems that even cancelling the request causes a form of leak :

The main differences are :

the amount of listeners not going through the roof
however "Nodes" do
the numbers are different, but the memory consumption is still going up

For testing this out, I basically went on the app's page and ran :

const clickOnLink = (href) => document.querySelector(`a[href="/${href}"]`).click()
setInterval(() => clickOnLink("users"), 1000);
setTimeout(() => setInterval(() => clickOnLink("posts"), 1500), 500);

Then I performed an analysis over 2+ minutes

Hundeyin Oluwadamilola • Oct 11 '20

your explaination helped me so much when fixing this issue!. thanks

Aleksandr Hovhannisyan • Jul 31 '20 • Edited

Note that this is actually an anti-pattern.

Nans Dumortier • Jul 31 '20 • Edited

As far as I understood this article from React, this.isMounted() in class based component is an anti pattern, in this case we are in a function component, and the hook I built doesn't use any this.isMounted() method.
It basically uses a hook like so :

function useIsComponentMounted() {
  const isMounted = useRef(false);
  useEffect(() => {
    isMounted.current = true;
    return () => isMounted.current = false;
  }, []);
  return isMounted;
};

Did I missunsertand anything?

Franciszek Krasnowski • Jul 31 '20

Yes, you did. What you just do is eliminating the warning, not solving anything. This is a good example of one of the worst attitudes to "solving" problems in programming. You will end up with a buggy program with no warning. Is that what you want?

Nans Dumortier • Jul 31 '20 • Edited

Heya, please do not be rough, we're being professional and debating!
You are actually right, thank you for pointing out 😉. I wrote another answer here showing the memory consumption :

Nans Dumortier

Jul 31

Yeah, thanks for your answer, I just went through some test, and must admit that this hook doesn't really solve the leak problem.
Here is a screenshot of the evidence :

So you, Aleksandr and Xavier are right, thank you guys for pointing out!

I must say that I was mistaken when reading through this - kind of famous - lib's code. Being downloaded 17000+ times a week, I thought it would use good practices.

I'm going to update the documentation and the article to add a disclaimer about that, and in the meantime I will try to find another elegant solution.

and will (soon) take measures to ensure this is not misused.

Aleksandr Hovhannisyan • Jul 31 '20

All good. I vaguely recall reading about a future React feature a while back that was supposed to solve this issue of memory leaks, but I can't remember what it was called. Something about a new way to make API calls.

Nans Dumortier • Jul 31 '20

I'm trying to investigate for a better and if possible elegant solution, if by any chance you get one, please share it with me ! 😄

Franciszek Krasnowski • Jul 31 '20

Yeah. I think the problem is much wider. There are so many misleading articles in this matter (canceling async request). For example '' JavaScript Promise is miraculously leakproof ", " There is no need for cleaning up after requests " - these are such common beliefs. I would call myself a newbie and I know how hard it is to trust to some resources

Nans Dumortier • Jul 31 '20

Yeah, I really feel deceived by all those articles and libs out there.
I must say I don't really know if there is a true good solution, since even AbortController doesn't seem to work ...
BTW I updated the article and invited the readers to come and check the discussions out 👍.

zNareak • Nov 18 '20 • Edited

Contributing to what the other people wrote, I have not found an effective and efficient solution either.
I have no idea how large applications maintain good performance in their asynchronous processes (such as promises or callbacks).
Perhaps it is the type of technology that is used that manages those leaks and prevents them.

With respect to famous libraries like react-async or react-query, I suppose they also make use of the isMounted antipattern.
There is no programmatic way to know when a component is unmounted, those libraries use that antipatron internally, they offer good features like data storage and state mahines. However, I'll give you some official links that go to the react-query repository, and analyze its code.

Using the antipattern:
github.com/tannerlinsley/react-que...

antipattern:
github.com/tannerlinsley/react-que...

Maybe the react-query code is more optimized for performance and also tested and proven, but they still use the anti-pattern methodology.

Thanks for the article, I have it in favorites to follow this thread, if you find a post on the react page how to solve this problem or you already find a solution, you can share it with us.
Thank you.

Nans Dumortier • Nov 18 '20

Woah I didn't expect such a great comment, thank you for contributing to this thread !

Let's hope we'll find a good solution one day !

Thank you again for your kind message.

mohantejach • Sep 11 '21 • Edited

I have a solution, but the promise chaining is not possible in this case. I used an EventEmitter (observer pattern) to attach and detach callbacks.

function asyncCaller(asyncFun, params, thenBlock, catchBlock) {
  const id = getRandomString(10);
  EventEmitterInstance.addListener(`promiseThen_${id}`, thenBlock);
  EventEmitterInstance.addListener(`promiseCatch_${id}`, catchBlock);
  const cancel = () => {
    EventEmitterInstance.removeAllListeners(`promiseThen_${id}`);
    EventEmitterInstance.removeAllListeners(`promiseCatch_${id}`);
  };
  asyncFun(...params)
    .then((resp) => {
      EventEmitterInstance.emit(`promiseThen_${id}`, resp);
    })
    .catch((error) => {
      EventEmitterInstance.emit(`promiseCatch_${id}`, error);
    }).finally(() => {
      // need to clear the listeners at the end
      cancel();
    });
  return cancel;
}

// in the component hook

React.useEffect(() => {
    const thenBlock = () => {
      setText("done!");
    };
    const catchBlock = (error) => {
      console.log(error);
    };
    const cancelCall = asyncCaller(
      simulateSlowNetworkRequest,
      [],
      thenBlock,
      catchBlock
    );
    return () => {
      cancelCall();
    };
  }, [setText]);

here is the working code link. I reused your example

mohantejach • Sep 17 '21 • Edited

an improved version of the above code : link

const PromiseObserver = new EventEmitter();

class AsyncAbort {
  constructor() {
    this.id = `async_${getRandomString(10)}`;
    this.asyncFun = null;
    this.asyncFunParams = [];
    this.thenBlock = null;
    this.catchBlock = null;
    this.finallyBlock = null;
  }

  addCall(asyncFun, params) {
    this.asyncFun = asyncFun;
    this.asyncFunParams = params;
    return this;
  }

  addThen(callback) {
    this.thenBlock = callback;
    return this;
  }

  addCatch(callback) {
    this.catchBlock = callback;
    return this;
  }

  addFinally(callback) {
    this.finallyBlock = callback;
    return this;
  }

  call() {
    const callback = ({ type, value }) => {
      switch (type) {
        case "then":
          if (this.thenBlock) this.thenBlock(value);
          break;
        case "catch":
          if (this.catchBlock) this.catchBlock(value);
          break;
        case "finally":
          if (this.finallyBlock) this.finallyBlock(value);
          break;
        default:
      }
    };
    PromiseObserver.addListener(this.id, callback);
    const cancel = () => {
      PromiseObserver.removeAllListeners(this.id);
    };
    this.asyncFun(...this.asyncFunParams)
      .then((resp) => {
        PromiseObserver.emit(this.id, { type: "then", value: resp });
      })
      .catch((error) => {
        PromiseObserver.emit(this.id, { type: "catch", value: error });
      })
      .finally(() => {
        PromiseObserver.emit(this.id, { type: "finally" });
        PromiseObserver.removeAllListeners(this.id);
      });
    return cancel;
  }
}

in the useEffect hook you can do

React.useEffect(() => {
    const abort = new AsyncAbort()
      .addCall(simulateSlowNetworkRequest, [])
      .addThen((resp) => {
        setText("done!");
      })
      .addCatch((error) => {
        console.log(error);
      })
      .call();
    return () => {
      abort();
    };
  }, [setText]);

Bryann Sotomayor-Rinaldi • May 26 '22

Wow, you're brilliant!

Roni Castro • Oct 30 '20

Hey @nans , did you find any solution that uses the request cancelation approach + hook to avoid calling setState after unmount?
I am also curious to know how you did these memory tests to figure out that this hook does not really solve the memory problem, can you share how did you do them?

Nans Dumortier • Oct 30 '20

Hey Roni !
My best guess right now would be to go for a popular library like react-query. I haven't personally tested it, but since it is quite largely adopted, I assume someone tested it !
For the memory tests, I simply used the Chrome Dev Tools' Performance Tab which enable you to record the memory footprint during a period of time.

Roni Castro • Oct 31 '20

Thanks @nans . I think you are right as the react-query ignore unresolved promises instead of cancelling (react-query.tanstack.com/docs/guid...), but it also allows to cancel it if the query is very expensive. I will try it out to check if It fixes the memory leak problem, but it probably fixes because there is no issue related to memory leak in their github repo. Also thanks for the tip, I didn't know Dev tools had this feature.

Wallace Nascimento Pereira • Aug 1 '20 • Edited

Have you already tried to use an asynchronous IIFE? Something like this:

useEffect (()=>{
  (async function Mount(){
      await action_1()
      await action_2()
    })();

   const unMount=()=>{}

   return unMount;
},[])

I usually use this approach on my useEffect hooks and I have no problems with memory leaks on my Components.

Nans Dumortier • Aug 1 '20

Thanks for your comment!
I don't think this would work, since if you update the state in action_1 or action_2 after the component has been unmounted, you will get the warning. Here is a codesandbox example :
codesandbox.io/s/trusting-mountain...
Or did I miss your point ?

meidikawardana • Jul 26 '21

This solution works for me. Don't ask me why, because I don't have time to inspect it more.

Xavier Brinon • Jul 31 '20

As much as I apprieciate the "elegant" solution, this is not solving the root of the pbm which should be how to cancel the request when the app doesn't need the response anymore.

Nans Dumortier • Jul 31 '20

You are actually right, thank you for pointing out !
This is my mistake. I got confused by reading too much bad practices online 🙈
I wrote another answer here showing the memory consumption :

Nans Dumortier • Jul 31

Yeah, thanks for your answer, I just went through some test, and must admit that this hook doesn't really solve the leak problem.
Here is a screenshot of the evidence :

So you, Aleksandr and Xavier are right, thank you guys for pointing out!

I must say that I was mistaken when reading through this - kind of famous - lib's code. Being downloaded 17000+ times a week, I thought it would use good practices.

I'm going to update the documentation and the article to add a disclaimer about that, and in the meantime I will try to find another elegant solution.