microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.6k stars 2.75k forks source link

Memory leaks caused by Sticky, ScrollablePane, and WithViewport #11810

Closed ThomasMichon closed 3 years ago

ThomasMichon commented 4 years ago

Various combinations of DetailsList with ScrollablePane, Sticky, and MarqueeSelection can lead to memory leaks because certain listeners and objects are not disposed even though the DetailsList itself might be unmounted from the React tree. This is likely due to closures captured in memoized callbacks, or simple failure to dispose event handlers.

Environment Information

Please provide a reproduction of the bug in a codepen:

https://codepen.io/ThomasMichon/pen/VwYNMwv

This demo reproduces a simple construct of ScrollablePane, MarqueeSelection, and DetailsList, and provides configuration options to make the memory leak appear and disappear. Note that taking heap snapshots in the correct iframe in CodePen is tricky, so pay attention when attempting to verify the issue and fixes.

Actual behavior:

After deconstructing the React tree for a DetailsList and its support components, instances of DetailsList are still left in the heap memory (after garbage collection).

Expected behavior:

All traces of DetailsList should be removed from memory after deconstructing the instance in React.

Priorities and help requested:

Are you willing to submit a PR to fix? Yes

Requested priority: High

Products/sites affected: (if applicable)

OneDrive, SharePoint, Microsoft Teams, File Picker

leddie24 commented 4 years ago

Hey @dzearing and @ThomasMichon, I'm not sure if this is a memory leak with ScrollablePane/Sticky or DetailsList. I've tried reproing this issue and using a different component (e.g: Toggle) instead of DetailsList/Row, etc and couldn't repro the memory leak issue that I can with Thomas's codepen.

I have a codepen here:

https://codepen.io/leddie24/pen/ZEGbYvq

I have screenshots of my heap snapshots below. The first image is when the ScrollablePane is visible, and the second screen is after I incremented the counter so the ScrollablePane is unmounted, and no traces of Toggle can be found.

image

image

Tagging @KevinTCoughlin for visibility.

msft-fluent-ui-bot commented 3 years ago

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.