React-popper: Memory leak when Reference children are removed from a page

Created on 1 Oct 2019  路  4Comments  路  Source: popperjs/react-popper

Reproduction demo

https://codesandbox.io/s/react-popper-v1x-issue-template-3hcp9

Steps to reproduce the problem

  1. Replace the demo code in the repo's demo/index.js file with the code in index.js of the codesandbox's index.js file
  2. Start the demo app and load it in Chrome
  3. Open Chrome devtools and take a heap snapshot
  4. Click on "Toggle Popout" in the demo page
  5. Take another heap snapshot
  6. In the second heap snapshot look for "Detached HTMLDivElement" with the shortest distance and find in the retainer panel that referenceNode is retaining the div element

What is the expected behavior?

referenceNode in Manager is set to null when the Reference element is removed from the page.

What went wrong?

When the Reference element is removed from the page, the ref value passed to the Manager context is retained. This creates a memory leak in situations where the reference element is regularly removed from a page.

Any other comments?

  • This is most easily seen in a production build of React since the dev build adds a lot of extra utilities that prevents a clear exposure of the leak.

Packages versions

  • Popper.js: whatever version react-popper 1.3.4 depends on
  • react-popper: 1.3.4

Most helpful comment

@FezVrasta Got a PR up for this. LMK if you want anything changed on it.

All 4 comments

It looks like the reason this is happening is because of this line in Manager.js: https://github.com/FezVrasta/react-popper/blob/master/src/Manager.js#L35

The Reference component sets its ref attribute to null when it is unmounting, but Manager no-ops when a null value is sent.

Thanks for the report! Would you mind if I asked you to send a PR with a proposed fix?

Already working on it :)

I should have one up later this morning

@FezVrasta Got a PR up for this. LMK if you want anything changed on it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

0xdevalias picture 0xdevalias  路  6Comments

vincerubinetti picture vincerubinetti  路  3Comments

websitesca picture websitesca  路  5Comments

giladgray picture giladgray  路  3Comments

souporserious picture souporserious  路  6Comments