Enzyme: unmount doesn't help to free memory

[denisgursky]

Hello,
My understanding is that there shouldn't be any references to the mounted component after an unmount call. But it doesn't work just like that. Here's my test:

class MyData {}

class TestComponent extends React.Component {
    render() {
        return <div></div>;
    }
}

describe("TestComponent", function() {
    afterAll(function() {
        debugger;
        //here taking the heap snapshot
    });

    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].forEach(function(num) {
        it("test " + num, function() {
            const myCmp = enzyme.mount(<TestComponent data={new MyData()}/>);
            myCmp.unmount();
            expect(true).toEqual(true);
        });
    });
});

The attached heap snapshot was made in the afterAll block. It clearly shows 10 instances of the MyData class, but I expect 0.

If I change the unmount call to:

ReactDOM.unmountComponentAtNode(ReactDOM.findDOMNode(myCmp.instance()).parentNode);

There're no MyData instances in the heap snapshot as expected.

Is it a bug in the unmount function? Or somewhere else. I'm not sure about the expected behavior here.

[ljharb]

Calling unmount doesn't garbage-collect myCmp, which is still in scope, and still holds a reference to the data prop.

JS is a memory-managed language, so the bug here is the programmer having any expectations about how memory works in it :-)

[the-spyke]

@ljharb Could you explain a little bit more? I see that snapshot is made in afterAll callback. It executes when specification callback is finished, so any scope or closure memory may be cleaned up. Moreover, taking memory snapshot in Chrome always triggers garbage collection

[ljharb]

What I mean is, in a memory-managed language, memory generally isn't something the programmer should be concerned about. They should care about it even less with a testing framework, which doesn't run in production.

What's the goal of taking a heap snapshot here in the first place?

[denisgursky]

@ljharb
I have a bunch of tests where props eat a good amount of memory. So I have to cleanup to avoid huge memory leak, because otherwise the browser crashes in the middle of my tests.

The heap snapshot is here because that's the only way to show what's going on there.

[the-spyke]

@ljharb Maybe you meant that in JS programmer shouldn't care about _how_ memory will be cleaned up, but that it _is_ available for cleaning?

If props contain much data, and a component instance hangs in memory, you may fail finishing tests with Out of Memory error. Taking snapshot allows you to understand where are your memory going. Also without proper clean up in tests you may not notice memory leakage in you own code.

[Vanuan]

Have same thing here too. Memory grows up to 2 Gb (I have 6 workers set for jest so it's actually 2x6 = 12 Gb) until it either finishes or is killed by OOM killer.
Maybe I don't use it properly?

Here's an example:

it('Should mount be cleaned up?', () => {
  const wrapper = mount(<MyComponentWithALotOfMemory />);
  expect(wrapper.find('span').first()).toBeTruthy();
  // should I unmount it here?
});

React documentation had this:

If you are rendering React components within a single-page app, you may need to plug into the app's view lifecycle to ensure your app will invoke unmountComponentAtNode at the appropriate time. React will not automatically clean up a tree. You need to manually call:

ReactDOM.unmountComponentAtNode(domContainerNode)

This is important and often forgotten. Forgetting to call unmountComponentAtNode will cause your app to leak memory. There is no way for us to automatically detect when it is appropriate to do this work. Every system is different.

And it makes sense since this method cleans up event handlers:

ReactDOM.unmountComponentAtNode

Remove a mounted React component from the DOM and clean up its event handlers and state. If no component was mounted in the container, calling this function does nothing. Returns true if a component was unmounted and false if there was no component to unmount.

And whenever you have an event handler for a DOM element it would prevent that element from being garbage collected since there might be an event at some point in the future. Although some browsers don't require event listeners to be removed, it might not be the case for a popular jsdom implementation. Am I missing something?

Enzyme states that unmountComponentAtNode should only be used if you use attachTo.

   * This method will most commonly be used as a "cleanup" method if you decide to use the
   * `attachTo` option in `mount(node, options)`.

However the code in master now have this for unmount method:

      unmount() {
        ReactDOM.unmountComponentAtNode(domNode);

So which is correct? Should I call unmount() or not? Should I use detach()?

[Lehakos]

@ljharb ping