Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ComboBox: Fix React.StrictMode issues #14719

Merged
merged 29 commits into from
Sep 17, 2020
Merged

ComboBox: Fix React.StrictMode issues #14719

merged 29 commits into from
Sep 17, 2020

Conversation

MLoughry
Copy link
Contributor

Pull request checklist

Description of changes

Partially migrate ComboBox to a functional component in order to mitigate React.StrictMode issues.

Focus areas to test

(optional)

@size-auditor
Copy link

size-auditor bot commented Aug 24, 2020 •
edited
Loading

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: a206613

Possible causes

  • The baseline build a206613 is broken
  • The Size Auditor run for the baseline build a206613 was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 24, 2020 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a7691e3:

Sandbox Source
Fluent UI Button Configuration
microsoft/fluentui: codesandbox-react-template Configuration
microsoft/fluentui: codesandbox-react-next-template Configuration

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Aug 24, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 959 990 5000
Breadcrumb mount 45573 44887 5000
BreadcrumbNext mount 167100 169837 5000
ButtonNext mount 629 634 5000
Checkbox mount 1721 1744 5000
CheckboxBase mount 1471 1490 5000
CheckboxNext mount 1643 1652 5000
ChoiceGroup mount 5462 5447 5000
ChoiceGroupNext mount 5431 5525 5000
ComboBox mount 991 1016 1000
CommandBar mount 8073 8160 1000
ContextualMenu mount 14090 14670 1000
DefaultButton mount 1178 1230 5000
DetailsRow mount 3826 3869 5000
DetailsRowFast mount 3936 3819 5000
DetailsRowNoStyles mount 3692 3646 5000
Dialog mount 1592 1595 1000
DocumentCardTitle mount 1931 1928 1000
Dropdown mount 2801 2921 5000
FocusZone mount 1915 1875 5000
IconButton mount 1936 1980 5000
Label mount 362 364 5000
Link mount 496 481 5000
LinkNext mount 498 515 5000
MenuButton mount 1568 1629 5000
MessageBar mount 2219 2204 5000
MessageBarNext mount 2166 2146 5000
Nav mount 3490 3532 1000
OverflowSet mount 1569 1503 5000
OverflowSetNext mount 1108 1074 5000
Panel mount 1534 1517 1000
Persona mount 889 884 1000
Pivot mount 1540 1526 1000
PivotNext mount 1502 1550 1000
PrimaryButton mount 1360 1392 5000
Rating mount 8349 8416 5000
RatingNext mount 8312 8357 5000
SearchBox mount 1392 1389 5000
SearchBoxNext mount 1480 1482 5000
Shimmer mount 2894 2929 5000
ShimmerNext mount 2824 2848 5000
Slider mount 1663 1650 5000
SliderNext mount 2112 2117 5000
SpinButton mount 5389 5331 5000
SpinButtonNext mount 5494 5476 5000
Spinner mount 454 478 5000
SplitButton mount 3364 3400 5000
Stack mount 581 554 5000
StackWithIntrinsicChildren mount 2064 2009 5000
StackWithTextChildren mount 5504 5445 5000
SwatchColorPicker mount 10950 11105 5000
SwatchColorPickerNext mount 11212 11054 5000
TagPicker mount 3025 3017 5000
TeachingBubble mount 54440 55077 5000
TeachingBubbleNext mount 54732 55343 5000
Text mount 482 500 5000
TextField mount 1545 1541 5000
ThemeProvider mount 5001 4992 5000
ThemeProvider virtual-rerender 570 552 5000
Toggle mount 910 946 5000
ToggleNext mount 867 906 5000
button mount 131 118 5000

Perf Analysis (Fluent)

⚠️ 5 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonOverridesMissPerf.default 1854 46 40.3:1 analysis
ButtonUseCssNestingPerf.default 1194 46 25.96:1 analysis
ButtonUseCssPerf.default 933 46 20.28:1 analysis
ChatWithPopoverPerf.default 517 497 1.04:1 analysis
ListNestedPerf.default 652 952 0.68:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.51 0.53 0.96:1 2000 1028
🦄 Button.Fluent 0.13 0.22 0.59:1 5000 653
🔧 Checkbox.Fluent 0.7 0.39 1.79:1 1000 698
🎯 Dialog.Fluent 0.18 0.23 0.78:1 5000 909
🔧 Dropdown.Fluent 3.12 0.55 5.67:1 1000 3118
🔧 Icon.Fluent 0.15 0.07 2.14:1 5000 760
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 451
🔧 Slider.Fluent 1.68 0.41 4.1:1 1000 1681
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 420
🦄 Tooltip.Fluent 0.12 17.82 0.01:1 5000 606

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 168 117 1.44:1
ImageMinimalPerf.default 489 377 1.3:1
RefMinimalPerf.default 252 198 1.27:1
AccordionMinimalPerf.default 186 155 1.2:1
DividerMinimalPerf.default 459 384 1.2:1
HeaderMinimalPerf.default 439 365 1.2:1
LabelMinimalPerf.default 509 424 1.2:1
RadioGroupMinimalPerf.default 533 445 1.2:1
SegmentMinimalPerf.default 428 363 1.18:1
CardMinimalPerf.default 675 584 1.16:1
GridMinimalPerf.default 389 336 1.16:1
TableMinimalPerf.default 485 421 1.15:1
IconMinimalPerf.default 804 703 1.14:1
Tooltip.Fluent 606 533 1.14:1
BoxMinimalPerf.default 421 373 1.13:1
StatusMinimalPerf.default 857 760 1.13:1
Text.Fluent 420 373 1.13:1
SkeletonMinimalPerf.default 488 435 1.12:1
AnimationMinimalPerf.default 453 407 1.11:1
ButtonMinimalPerf.default 207 186 1.11:1
FlexMinimalPerf.default 356 321 1.11:1
FormMinimalPerf.default 496 446 1.11:1
TooltipMinimalPerf.default 932 838 1.11:1
AttachmentMinimalPerf.default 181 164 1.1:1
ChatMinimalPerf.default 702 639 1.1:1
ProviderMinimalPerf.default 1033 941 1.1:1
ReactionMinimalPerf.default 462 419 1.1:1
Button.Fluent 653 592 1.1:1
ListMinimalPerf.default 551 506 1.09:1
PopupMinimalPerf.default 776 715 1.09:1
ProviderMergeThemesPerf.default 2070 1898 1.09:1
TextAreaMinimalPerf.default 560 515 1.09:1
Avatar.Fluent 1028 947 1.09:1
Dialog.Fluent 909 836 1.09:1
Image.Fluent 451 415 1.09:1
EmbedMinimalPerf.default 2231 2069 1.08:1
ToolbarMinimalPerf.default 1135 1053 1.08:1
HeaderSlotsPerf.default 932 867 1.07:1
LayoutMinimalPerf.default 480 447 1.07:1
MenuButtonMinimalPerf.default 1787 1674 1.07:1
TextMinimalPerf.default 410 384 1.07:1
ChatDuplicateMessagesPerf.default 459 431 1.06:1
DropdownManyItemsPerf.default 842 794 1.06:1
TreeMinimalPerf.default 1011 956 1.06:1
TreeWith60ListItems.default 230 216 1.06:1
AvatarMinimalPerf.default 543 518 1.05:1
SplitButtonMinimalPerf.default 4105 3910 1.05:1
AlertMinimalPerf.default 343 329 1.04:1
ButtonSlotsPerf.default 662 636 1.04:1
DialogMinimalPerf.default 856 826 1.04:1
MenuMinimalPerf.default 966 927 1.04:1
CustomToolbarPrototype.default 4018 3850 1.04:1
Checkbox.Fluent 698 669 1.04:1
InputMinimalPerf.default 1426 1400 1.02:1
VideoMinimalPerf.default 689 673 1.02:1
Dropdown.Fluent 3118 3048 1.02:1
CheckboxMinimalPerf.default 3046 3013 1.01:1
ItemLayoutMinimalPerf.default 1429 1418 1.01:1
TableManyItemsPerf.default 2465 2431 1.01:1
Slider.Fluent 1681 1666 1.01:1
AttachmentSlotsPerf.default 1275 1274 1:1
LoaderMinimalPerf.default 801 805 1:1
SliderMinimalPerf.default 1603 1604 1:1
DropdownMinimalPerf.default 3099 3127 0.99:1
Icon.Fluent 760 767 0.99:1
CarouselMinimalPerf.default 479 491 0.98:1
ListWith60ListItems.default 1029 1187 0.87:1
ListCommonPerf.default 748 979 0.76:1

@MLoughry MLoughry requested a review from ecraig12345 August 24, 2020 20:19
@MLoughry MLoughry closed this Sep 1, 2020
@MLoughry MLoughry reopened this Sep 1, 2020
@joschect joschect self-requested a review September 4, 2020 18:30
@MLoughry MLoughry merged commit 2a65fb9 into microsoft:master Sep 17, 2020
@ecraig12345 ecraig12345 mentioned this pull request Sep 17, 2020
Merged
@msft-github-bot
Copy link
Contributor

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot msft-github-bot mentioned this pull request Sep 18, 2020
Closed
@msft-github-bot
Copy link
Contributor

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@ecraig12345 ecraig12345 mentioned this pull request Oct 13, 2020
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ecraig12345 ecraig12345 ecraig12345 left review comments

@joschect joschect joschect approved these changes

@bigbadcapers bigbadcapers Awaiting requested review from bigbadcapers

@Jahnp Jahnp Awaiting requested review from Jahnp

Assignees
No one assigned
Labels
Component: Combobox PR: API Modified PR: Possible Perf Regression 🐌
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ComboBox: strict mode compliance (UNSAFE_)
4 participants
@MLoughry @msft-github-bot @ecraig12345 @joschect