DEV Community

Cover image for An elegant solution for memory leaks in React

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...
Collapse
 
steveruizok profile image
Steve Ruiz

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.
Collapse
 
nans profile image
Nans Dumortier

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 :
a graph showing memory consumption over time, that grows

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.

Collapse
 
nans profile image
Nans Dumortier • 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 :
a graph with memory consumption going up
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

Collapse
 
dbankx profile image
Hundeyin Oluwadamilola

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

Collapse
 
aleksandrhovhannisyan profile image
Aleksandr Hovhannisyan • Edited
Collapse
 
nans profile image
Nans Dumortier • 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?

Collapse
 
fkrasnowski profile image
Franciszek Krasnowski

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?

Thread Thread
 
nans profile image
Nans Dumortier • 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 :

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 :
a graph showing memory consumption over time, that grows

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.
Thread Thread
 
aleksandrhovhannisyan profile image
Aleksandr Hovhannisyan

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.

Thread Thread
 
nans profile image
Nans Dumortier

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 ! 😄

Thread Thread
 
fkrasnowski profile image
Franciszek Krasnowski

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

Thread Thread
 
nans profile image
Nans Dumortier

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 👍.

Collapse
 
znareak profile image
zNareak • 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.

Collapse
 
nans profile image
Nans Dumortier

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.

Collapse
 
mohantejach profile image
mohantejach • 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]);

Enter fullscreen mode Exit fullscreen mode

here is the working code link. I reused your example

Collapse
 
mohantejach profile image
mohantejach • 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;
  }
}

Enter fullscreen mode Exit fullscreen mode

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]);
Enter fullscreen mode Exit fullscreen mode
Collapse
 
sotobry profile image
Bryann Sotomayor-Rinaldi

Wow, you're brilliant!

Collapse
 
ronicastro profile image
Roni Castro

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?

Collapse
 
nans profile image
Nans Dumortier

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.

Collapse
 
ronicastro profile image
Roni Castro

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.

Collapse
 
wlcpereira profile image
Wallace Nascimento Pereira • 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.

Collapse
 
nans profile image
Nans Dumortier

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 ?

Collapse
 
meidikawardana profile image
meidikawardana

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

Collapse
 
xavierbrinonecs profile image
Xavier Brinon

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.

Collapse
 
nans profile image
Nans Dumortier

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 :

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 :
a graph showing memory consumption over time, that grows

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.

</div>
Enter fullscreen mode Exit fullscreen mode



and will take measures to ensure this is not misused.
Collapse
 
0916dhkim profile image
Danny Kim

I think cancel token of axios is an actual solution for this problem. What's your thoughts about request cancellation during useEffect cleanup?

Collapse
 
kyimoemin profile image
Kyi-Moe-Min

I think your solution will work if you use useRef to isMounted variable.

Collapse
 
mesvil7 profile image
Maikel Espitia • Edited

A solution can be manage the sideEffect with redux Thunk or Saga

Collapse
 
mohantejach profile image
mohantejach

yes, we don't face this problem if we use redux