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

fix(Ref): do not return Fibers as refs for "react-test-renderer" #15144

Merged
merged 2 commits into from
Sep 21, 2020
Merged

fix(Ref): do not return Fibers as refs for "react-test-renderer" #15144

merged 2 commits into from
Sep 21, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Sep 21, 2020 •
edited
Loading

This PR fixes an issue that can lead us to very weird bugs (Semantic-Org/Semantic-UI-React#4061) as ReactDOM.findDOMNode() for react-test-renderer will return a React Fiber facebook/react#7371 (comment).

@msft-github-bot msft-github-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Sep 21, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 21, 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 29f4cdc:

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

@size-auditor
Copy link

size-auditor bot commented Sep 21, 2020 •
edited
Loading

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 1bbb6a00cc283b1b1eeecf953595f962eb93f532 (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Sep 21, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 879 856 5000
Breadcrumb mount 41935 41882 5000
BreadcrumbNext mount 160869 159598 5000
ButtonNext mount 570 595 5000
Checkbox mount 1592 1532 5000
CheckboxBase mount 1298 1305 5000
CheckboxNext mount 1511 1502 5000
ChoiceGroup mount 4944 4936 5000
ChoiceGroupNext mount 4951 4944 5000
ComboBox mount 935 929 1000
CommandBar mount 7565 7590 1000
ContextualMenu mount 13709 13545 1000
DefaultButton mount 1100 1086 5000
DetailsRow mount 3529 3563 5000
DetailsRowFast mount 3527 3504 5000
DetailsRowNoStyles mount 3250 3339 5000
Dialog mount 1497 1481 1000
DocumentCardTitle mount 1838 1828 1000
Dropdown mount 2545 2571 5000
FocusZone mount 1796 1838 5000
IconButton mount 1713 1731 5000
Label mount 345 355 5000
Layer mount 1946 1964 5000
LayerNext mount 1892 1891 5000
Link mount 457 447 5000
LinkNext mount 463 466 5000
MenuButton mount 1415 1400 5000
MessageBar mount 2081 2062 5000
MessageBarNext mount 2011 2025 5000
Nav mount 3249 3212 1000
OverflowSet mount 1388 1401 5000
OverflowSetNext mount 1041 1065 5000
Panel mount 1444 1507 1000
Persona mount 854 835 1000
Pivot mount 1399 1426 1000
PivotNext mount 1398 1368 1000
PrimaryButton mount 1274 1234 5000
Rating mount 7607 7595 5000
RatingNext mount 7451 7367 5000
SearchBox mount 1274 1231 5000
SearchBoxNext mount 1293 1295 5000
Shimmer mount 2522 2528 5000
ShimmerNext mount 2497 2503 5000
Slider mount 1521 1491 5000
SliderNext mount 1898 1888 5000
SpinButton mount 4911 4908 5000
SpinButtonNext mount 4976 5081 5000
Spinner mount 445 431 5000
SplitButton mount 3129 3109 5000
Stack mount 522 541 5000
StackWithIntrinsicChildren mount 1982 1974 5000
StackWithTextChildren mount 4972 4975 5000
SwatchColorPicker mount 10228 11234 5000
SwatchColorPickerNext mount 10217 11095 5000
TagPicker mount 2748 2681 5000
TeachingBubble mount 51257 50609 5000
TeachingBubbleNext mount 50832 50492 5000
Text mount 414 423 5000
TextField mount 1357 1365 5000
ThemeProvider mount 2473 2470 5000
ThemeProvider virtual-rerender 606 598 5000
Toggle mount 816 830 5000
ToggleNext mount 818 801 5000
button mount 130 137 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 1719 41 41.93:1 analysis
ButtonUseCssNestingPerf.default 1120 43 26.05:1 analysis
ButtonUseCssPerf.default 816 46 17.74:1 analysis
ChatWithPopoverPerf.default 497 466 1.07:1 analysis
ListCommonPerf.default 656 928 0.71:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.45 0.48 0.94:1 2000 893
🦄 Button.Fluent 0.12 0.19 0.63:1 5000 577
🔧 Checkbox.Fluent 0.66 0.34 1.94:1 1000 657
🎯 Dialog.Fluent 0.16 0.23 0.7:1 5000 806
🔧 Dropdown.Fluent 2.93 0.49 5.98:1 1000 2931
🔧 Icon.Fluent 0.14 0.06 2.33:1 5000 712
🎯 Image.Fluent 0.08 0.11 0.73:1 5000 408
🔧 Slider.Fluent 1.6 0.37 4.32:1 1000 1600
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 377
🦄 Tooltip.Fluent 0.11 17.05 0.01:1 5000 561

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 162 114 1.42:1
RefMinimalPerf.default 255 204 1.25:1
FlexMinimalPerf.default 333 268 1.24:1
StatusMinimalPerf.default 773 637 1.21:1
HeaderMinimalPerf.default 393 334 1.18:1
Text.Fluent 377 319 1.18:1
GridMinimalPerf.default 378 322 1.17:1
ReactionMinimalPerf.default 432 368 1.17:1
VideoMinimalPerf.default 669 577 1.16:1
Image.Fluent 408 352 1.16:1
BoxMinimalPerf.default 374 324 1.15:1
LabelMinimalPerf.default 436 379 1.15:1
SkeletonMinimalPerf.default 444 387 1.15:1
ButtonMinimalPerf.default 194 170 1.14:1
DividerMinimalPerf.default 390 342 1.14:1
FormMinimalPerf.default 435 380 1.14:1
ImageMinimalPerf.default 410 361 1.14:1
TextMinimalPerf.default 376 329 1.14:1
AccordionMinimalPerf.default 163 144 1.13:1
RadioGroupMinimalPerf.default 448 398 1.13:1
SegmentMinimalPerf.default 373 331 1.13:1
TableMinimalPerf.default 437 388 1.13:1
AnimationMinimalPerf.default 427 381 1.12:1
LayoutMinimalPerf.default 426 382 1.12:1
PopupMinimalPerf.default 720 648 1.11:1
ListMinimalPerf.default 501 455 1.1:1
TooltipMinimalPerf.default 846 766 1.1:1
Tooltip.Fluent 561 511 1.1:1
CardMinimalPerf.default 577 527 1.09:1
ChatMinimalPerf.default 648 596 1.09:1
IconMinimalPerf.default 686 631 1.09:1
Button.Fluent 577 527 1.09:1
HeaderSlotsPerf.default 805 747 1.08:1
TextAreaMinimalPerf.default 485 448 1.08:1
Dialog.Fluent 806 748 1.08:1
DialogMinimalPerf.default 803 747 1.07:1
MenuMinimalPerf.default 871 817 1.07:1
ProviderMergeThemesPerf.default 2053 1918 1.07:1
ProviderMinimalPerf.default 976 912 1.07:1
Checkbox.Fluent 657 614 1.07:1
Icon.Fluent 712 663 1.07:1
ButtonSlotsPerf.default 610 575 1.06:1
MenuButtonMinimalPerf.default 1567 1473 1.06:1
TreeMinimalPerf.default 899 846 1.06:1
AttachmentMinimalPerf.default 162 154 1.05:1
AvatarMinimalPerf.default 487 463 1.05:1
CarouselMinimalPerf.default 479 455 1.05:1
ChatDuplicateMessagesPerf.default 436 417 1.05:1
LoaderMinimalPerf.default 759 722 1.05:1
ToolbarMinimalPerf.default 956 910 1.05:1
TreeWith60ListItems.default 219 209 1.05:1
Avatar.Fluent 893 854 1.05:1
CheckboxMinimalPerf.default 2898 2787 1.04:1
EmbedMinimalPerf.default 1956 1885 1.04:1
InputMinimalPerf.default 1327 1270 1.04:1
ItemLayoutMinimalPerf.default 1269 1226 1.04:1
CustomToolbarPrototype.default 3857 3708 1.04:1
SplitButtonMinimalPerf.default 3777 3659 1.03:1
AlertMinimalPerf.default 300 295 1.02:1
AttachmentSlotsPerf.default 1156 1128 1.02:1
DropdownManyItemsPerf.default 772 761 1.01:1
SliderMinimalPerf.default 1641 1624 1.01:1
TableManyItemsPerf.default 2156 2138 1.01:1
DropdownMinimalPerf.default 2957 2964 1:1
Dropdown.Fluent 2931 2918 1:1
Slider.Fluent 1600 1618 0.99:1
ListWith60ListItems.default 944 1091 0.87:1
ListNestedPerf.default 587 848 0.69:1

assuncaocharles
assuncaocharles approved these changes Sep 21, 2020
View reviewed changes
Copy link
Contributor

@assuncaocharles assuncaocharles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@layershifter layershifter merged commit bd424da into master Sep 21, 2020
@layershifter layershifter deleted the fix/ref-tr branch September 21, 2020 15:29
layershifter added a commit that referenced this pull request Sep 22, 2020
@layershifter
fix(Ref): do not return Fibers as refs for "react-test-renderer" (#15144
20072c1 
)

(cherry picked from commit bd424da)
layershifter added a commit that referenced this pull request Sep 22, 2020
@layershifter
fix(Ref): do not return Fibers as refs for "react-test-renderer" (#15144
4011558 
)

(cherry picked from commit bd424da)
@layershifter layershifter mentioned this pull request Sep 23, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@assuncaocharles assuncaocharles assuncaocharles approved these changes

@pompomon pompomon pompomon approved these changes

@dzearing dzearing dzearing approved these changes

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

Assignees

@miroslavstastny miroslavstastny

Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@layershifter @msft-github-bot @dzearing @pompomon @assuncaocharles @miroslavstastny