Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(FocusZone): Cleanup DOM element references on unmount #2306

Merged
merged 6 commits into from
Feb 3, 2020

Conversation

miroslavstastny
Copy link
Member

@miroslavstastny miroslavstastny commented Feb 2, 2020

The FocusZone component holds references to _activeElement and _defaultFocusElement. These references should be nulled out when the component is unmounted to prevent memory leaks.

Fixes #2303

Note: this was just moved from @johannao76 fork, all credit goes to her.

…unted

The FocusZone component holds references to _activeElement and _defaultFocusElement. These references should be nulled out when the component is unmounted to prevent memory leaks.

closes #2303

(cherry picked from commit 6a9282f)
(cherry picked from commit b31cf02)
(cherry picked from commit 8024cd0)
(cherry picked from commit 1ded3ad)
@miroslavstastny miroslavstastny changed the title fix(FocusZone): Cleanup DOM element references on unmount #2304 fix(FocusZone): Cleanup DOM element references on unmount Feb 2, 2020
CHANGELOG.md Outdated Show resolved Hide resolved
@DustyTheBot
Copy link
Collaborator

DustyTheBot commented Feb 2, 2020

Perf comparison

Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.55 0.39 1.41:1 2000 1092
🔧 Button.Fluent 1.26 0.17 7.41:1 1000 1261
🔧 Checkbox.Fluent 1.6 0.28 5.71:1 1000 1604
🔧 Dialog.Fluent 0.36 0.16 2.25:1 5000 1776
🔧 Dropdown.Fluent 3.49 0.37 9.43:1 1000 3486
🔧 Icon.Fluent 0.25 0.03 8.33:1 5000 1230
🔧 Image.Fluent 0.09 0.08 1.13:1 5000 474
🔧 Slider.Fluent 2.14 0.38 5.63:1 1000 2144
🦄 Text.Fluent 0.06 0.18 0.33:1 5000 288
🦄 Tooltip.Fluent 0.36 17.86 0.02:1 5000 1806

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ToolbarMinimalPerf.default 832 665 1.25:1
AccordionMinimalPerf.default 208 179 1.16:1
Checkbox.Fluent 1604 1433 1.12:1
DropdownManyItemsPerf.default 504 454 1.11:1
AttachmentMinimalPerf.default 989 901 1.1:1
ChatWithPopoverPerf.default 587 541 1.09:1
Dialog.Fluent 1776 1652 1.08:1
ListCommonPerf.default 1119 1042 1.07:1
SegmentMinimalPerf.default 1286 1217 1.06:1
FlexMinimalPerf.default 328 313 1.05:1
RadioGroupMinimalPerf.default 401 386 1.04:1
StatusMinimalPerf.default 918 885 1.04:1
TableMinimalPerf.default 554 533 1.04:1
Button.Fluent 1261 1215 1.04:1
Icon.Fluent 1230 1187 1.04:1
ProviderMinimalPerf.default 578 563 1.03:1
BoxMinimalPerf.default 203 199 1.02:1
ChatDuplicateMessagesPerf.default 535 522 1.02:1
DropdownMinimalPerf.default 3315 3259 1.02:1
MenuMinimalPerf.default 1932 1896 1.02:1
Dropdown.Fluent 3486 3413 1.02:1
AlertMinimalPerf.default 537 534 1.01:1
ButtonMinimalPerf.default 1237 1222 1.01:1
CarouselMinimalPerf.default 1759 1750 1.01:1
CheckboxMinimalPerf.default 6510 6442 1.01:1
GridMinimalPerf.default 760 750 1.01:1
HeaderMinimalPerf.default 409 406 1.01:1
IconMinimalPerf.default 1055 1044 1.01:1
ItemLayoutMinimalPerf.default 1579 1562 1.01:1
PortalMinimalPerf.default 244 241 1.01:1
TextMinimalPerf.default 247 245 1.01:1
AttachmentSlotsPerf.default 3174 3177 1:1
FormMinimalPerf.default 681 683 1:1
InputMinimalPerf.default 912 916 1:1
PopupMinimalPerf.default 325 325 1:1
ProviderMergeThemesPerf.default 1069 1066 1:1
SliderMinimalPerf.default 1848 1848 1:1
SplitButtonMinimalPerf.default 11715 11662 1:1
TooltipMinimalPerf.default 2089 2085 1:1
AvatarMinimalPerf.default 574 581 0.99:1
ButtonSlotsPerf.default 1649 1668 0.99:1
ChatMinimalPerf.default 1539 1560 0.99:1
DialogMinimalPerf.default 1586 1606 0.99:1
LayoutMinimalPerf.default 518 522 0.99:1
RefMinimalPerf.default 148 149 0.99:1
TextAreaMinimalPerf.default 2912 2950 0.99:1
TreeMinimalPerf.default 846 856 0.99:1
DividerMinimalPerf.default 823 840 0.98:1
EmbedMinimalPerf.default 6034 6157 0.98:1
ListMinimalPerf.default 638 650 0.98:1
Text.Fluent 288 294 0.98:1
HeaderSlotsPerf.default 1213 1246 0.97:1
HierarchicalTreeMinimalPerf.default 708 729 0.97:1
LabelMinimalPerf.default 1609 1680 0.96:1
Slider.Fluent 2144 2230 0.96:1
Tooltip.Fluent 1806 1879 0.96:1
Avatar.Fluent 1092 1148 0.95:1
MenuButtonMinimalPerf.default 1409 1506 0.94:1
ReactionMinimalPerf.default 2472 2694 0.92:1
AnimationMinimalPerf.default 421 464 0.91:1
ImageMinimalPerf.default 462 526 0.88:1
LoaderMinimalPerf.default 2148 2451 0.88:1
VideoMinimalPerf.default 671 769 0.87:1
CustomToolbarPrototype.default 3646 4498 0.81:1
Image.Fluent 474 600 0.79:1

Generated by 🚫 dangerJS

@layershifter layershifter added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. labels Feb 3, 2020
@miroslavstastny miroslavstastny merged commit 2cbf4ad into master Feb 3, 2020
@miroslavstastny miroslavstastny deleted the fix/focus-zone-leak branch February 3, 2020 09:27
miroslavstastny added a commit that referenced this pull request Feb 3, 2020
* fix(FocusZone): Cleanup DOM element references when component is unmounted

The FocusZone component holds references to _activeElement and _defaultFocusElement. These references should be nulled out when the component is unmounted to prevent memory leaks.

closes #2303

(cherry picked from commit 6a9282f)

* Change log update

(cherry picked from commit b31cf02)

* Fix typos

(cherry picked from commit 8024cd0)

* One more typo

(cherry picked from commit 1ded3ad)

* fixed changelog order

Co-authored-by: Johanna Hawkins <jo.hawkins@hotmail.com>
(cherry picked from commit 2cbf4ad)
miroslavstastny added a commit that referenced this pull request Feb 3, 2020
* fix(FocusZone): Cleanup DOM element references when component is unmounted

The FocusZone component holds references to _activeElement and _defaultFocusElement. These references should be nulled out when the component is unmounted to prevent memory leaks.

closes #2303

(cherry picked from commit 6a9282f)

* Change log update

(cherry picked from commit b31cf02)

* Fix typos

(cherry picked from commit 8024cd0)

* One more typo

(cherry picked from commit 1ded3ad)

* fixed changelog order

Co-authored-by: Johanna Hawkins <jo.hawkins@hotmail.com>
(cherry picked from commit 2cbf4ad)
@miroslavstastny miroslavstastny mentioned this pull request Feb 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leaks in FocusZone due to DOM object references not getting cleaned up
4 participants