Incorrect documentation for fixing memory leaks

[jadengeller-asana]

The "isMounted is an Antipattern" documentation article suggest avoiding memory leaks with makeCancelable, but this doesn't actually solve the issue since the callback is still referenced. The garbage collector does not understand that setting hasCanceled_ means that resolve will never be called, so it cannot release resolve or the references contained within. Even if it could, reject might also reference this, so memory leaks could still exist. Instead, makeCancelable should store the resolve and reject callbacks itself, and it should null them out on cancelation.

Another user pointed this issue out in facebook/react#5465 (comment)

[jinhale]

Is this anywhere near the mark? I'm having trouble understanding what you mean by "null them out".

export const makeCancelablePromise = (promise) => {
    let wrappedPromise = new Promise((resolve, reject) => {
        promise.then(
            val => resolve(val),
            error => reject(error),
        );
    });

    return {
        promise: wrappedPromise,
        cancel() {
            wrappedPromise = () => {};
        },
    };
};

[gaearon]

This blog post recommendation is a mess. 😛 These points are correct. Can you please send a PR to fix it?

[jadengeller-asana]

There are two pretty reasonable approaches I can think of for modifying this. Thoughts?

1. Instead of capturing the component directly in the promise, capture a "handle" that stores the component as a property. When the component is unmounted, null out this property to break the reference.

const componentHandle = new Handle(component);
promise.then(() => {
  if (!componentHandle.isReleased()) {
    const component = componentHandle.value();
  }
})

2. Register all callbacks with the component, wrapping them in functions that call the stored implementation. On unmount, cancel all registered callbacks, which deletes the functions stored within these wrappers.

promise.then(component.registeredCallback(() => {
  // directly reference component
}))

[pungggi]

see https://github.com/hjylewis/trashable-react

[ThekhoN]

I am using an older version of React - ^16.1.1 in our production.
where we get the following message,
"Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op."

Without resorting to any other dependencies, I'm able to resolve this issue by using refs.

// use ref to check if mounted and update state
    if (!this.leftNavTab) {
      return;
    } else {
      this.setState({
        height: updatedHeight
      });
    }

i've tested the same in the Latest Stable Version of React 16.8.3.

You can check this sandbox.
https://codesandbox.io/s/l9v08q509q
Check the LeftNavTab Component in /left-nav-tab
// calculating state.height based on window.resize

// How to reproduce issue:

1. disable ref check (i.e. update state directly)
2. save pen and refresh the page
3. open console, click on the LeftNavLinks and do random vertical resizes.
4. 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 the componentWillUnmount method."

// How to fix:

1. Enable ref check.
   Try steps 2 - 3
   No Warning is thrown this time.

[pungggi]

Dont you just hide the problem like this?

[ThekhoN]

i am attempting to avoid the "Can only update a mounted or mounting component.." warning, believing that React is able to effectively garbage collect if i'm not updating/setting state on an unmounted component.
In the example that i provided, i tried to do the this._isMounted check inside the constructor, but that still does NOT prevent the warning from showing up.