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

Merge react-internal back into react #16832

Merged
merged 14 commits into from
Feb 9, 2021
Merged

Merge react-internal back into react #16832

merged 14 commits into from
Feb 9, 2021

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Feb 4, 2021 •
edited
Loading

Per the discussion on #16577 and #16772 and elsewhere, we decided to get rid of @fluentui/react-internal since it's no longer strictly needed, and merge the contents back into @fluentui/react.

The one awkward thing about this is that the newer Calendar/DatePicker which have always lived in the @fluentui/react-date-time package were using things from @fluentui/react-internal, but having that package depend on @fluentui/react would cause a circular dep now that we export the newer Calendar/DatePicker by default.

Workaround is to move the Calendar and DatePicker implementations into @fluentui/react and have @fluentui/react-date-time re-export them. This is kind of awkward, but it works, and the change will be invisible to consumers unless they're using deep imports.

(PR is giant but the commits are somewhat organized.)

@ecraig12345
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 4, 2021 •
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 286a565:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@size-auditor
Copy link

size-auditor bot commented Feb 4, 2021 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Styling New   22.156 kB ExceedsTolerance     22.156 kB
office-ui-fabric-react fluentui-react-Layer New   247 bytes ExceedsBaseline     247 bytes
office-ui-fabric-react fluentui-react-WindowProvider New   247 bytes ExceedsBaseline     247 bytes
office-ui-fabric-react fluentui-react-Link New   247 bytes ExceedsBaseline     247 bytes
office-ui-fabric-react fluentui-react-Utilities New   247 bytes ExceedsBaseline     247 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: e3821c337deb9b6e1cff0df227066cfa2769c3a1 (build)

khmakoto
khmakoto approved these changes Feb 5, 2021
View reviewed changes
Copy link
Member

@khmakoto khmakoto left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I left one super small comment but otherwise I've approved the PR!

@JustSlone
Copy link
Collaborator

JustSlone commented Feb 8, 2021 •
edited
Loading

@ecraig12345 Just FYI this is going to collide with the pending work to add range support to Slider by @rockcs1992 (#16854)

@ecraig12345 ecraig12345 requested a review from Raghurk as a code owner February 9, 2021 08:04
@fabricteam
Copy link
Collaborator

fabricteam commented Feb 9, 2021 •
edited
Loading

Perf Analysis

Scenario Render type Master Ticks PR Ticks Iterations Status
buttonNative mount 113 107 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 821 826 5000
BaseButtonCompat mount 938 921 5000
Breadcrumb mount 40887 41343 5000
Checkbox mount 1566 1566 5000
CheckboxBase mount 1320 1325 5000
ChoiceGroup mount 4840 4922 5000
ComboBox mount 946 966 1000
CommandBar mount 10072 9823 1000
ContextualMenu mount 5955 6030 1000
DefaultButtonCompat mount 1180 1128 5000
DetailsRow mount 3724 3687 5000
DetailsRowFast mount 3653 3601 5000
DetailsRowNoStyles mount 3561 3456 5000
Dialog mount 1463 1429 1000
DocumentCardTitle mount 1767 1774 1000
Dropdown mount 3374 3377 5000
FocusTrapZone mount 1782 1871 5000
FocusZone mount 1779 1769 5000
IconButtonCompat mount 2049 1820 5000
Label mount 338 327 5000
Layer mount 1785 1847 5000
Link mount 482 481 5000
MakeStyles mount 1908 1907 50000
MenuButtonCompat mount 1548 1572 5000
MessageBar mount 1960 1941 5000
Nav mount 3350 3316 1000
OverflowSet mount 1027 1024 5000
Panel mount 1396 1438 1000
Persona mount 867 871 1000
Pivot mount 1414 1396 1000
PrimaryButtonCompat mount 1311 1275 5000
Rating mount 7843 7770 5000
SearchBox mount 1357 1353 5000
Shimmer mount 2624 2574 5000
Slider mount 1919 1913 5000
SpinButton mount 5023 5049 5000
Spinner mount 406 409 5000
SplitButtonCompat mount 3198 3223 5000
Stack mount 516 519 5000
StackWithIntrinsicChildren mount 1636 1628 5000
StackWithTextChildren mount 4675 4711 5000
SwatchColorPicker mount 10301 10417 5000
Tabs mount 1406 1429 1000
TagPicker mount 2867 2901 5000
TeachingBubble mount 11524 11503 5000
Text mount 417 426 5000
TextField mount 1381 1405 5000
ThemeProvider mount 1381 1368 5000
ThemeProvider virtual-rerender 585 586 5000
ThemeProviderNext mount 2120 2152 5000
Toggle mount 790 833 5000
button mount 693 701 5000
buttonNative mount 113 107 5000 Possible regression

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
PortalMinimalPerf.default 165 168 0.98:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.18 0.52 0.35:1 2000 369
🦄 Button.Fluent 0.13 0.21 0.62:1 5000 625
🔧 Checkbox.Fluent 0.65 0.37 1.76:1 1000 648
🎯 Dialog.Fluent 0.17 0.23 0.74:1 5000 835
🔧 Dropdown.Fluent 3.02 0.42 7.19:1 1000 3020
🔧 Icon.Fluent 0.14 0.07 2:1 5000 718
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 448
🔧 Slider.Fluent 1.61 0.43 3.74:1 1000 1614
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 409
🦄 Tooltip.Fluent 0.12 0.87 0.14:1 5000 580

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AvatarMinimalPerf.default 233 206 1.13:1
ButtonMinimalPerf.default 217 193 1.12:1
ReactionMinimalPerf.default 470 431 1.09:1
SkeletonMinimalPerf.default 427 397 1.08:1
TextMinimalPerf.default 418 391 1.07:1
GridMinimalPerf.default 399 378 1.06:1
AttachmentMinimalPerf.default 186 178 1.04:1
CardMinimalPerf.default 620 599 1.04:1
FormMinimalPerf.default 480 463 1.04:1
HeaderSlotsPerf.default 884 853 1.04:1
ListMinimalPerf.default 573 549 1.04:1
TooltipMinimalPerf.default 856 826 1.04:1
CarouselMinimalPerf.default 519 502 1.03:1
DialogMinimalPerf.default 851 826 1.03:1
DividerMinimalPerf.default 428 415 1.03:1
DropdownManyItemsPerf.default 788 762 1.03:1
FlexMinimalPerf.default 346 335 1.03:1
InputMinimalPerf.default 1348 1313 1.03:1
ItemLayoutMinimalPerf.default 1343 1306 1.03:1
LoaderMinimalPerf.default 758 735 1.03:1
PopupMinimalPerf.default 727 709 1.03:1
RefMinimalPerf.default 261 253 1.03:1
StatusMinimalPerf.default 799 779 1.03:1
Text.Fluent 409 397 1.03:1
AccordionMinimalPerf.default 173 170 1.02:1
AlertMinimalPerf.default 316 311 1.02:1
AnimationMinimalPerf.default 431 424 1.02:1
ImageMinimalPerf.default 447 439 1.02:1
LayoutMinimalPerf.default 453 445 1.02:1
SegmentMinimalPerf.default 404 396 1.02:1
SplitButtonMinimalPerf.default 3940 3852 1.02:1
IconMinimalPerf.default 719 707 1.02:1
TableMinimalPerf.default 458 449 1.02:1
TextAreaMinimalPerf.default 563 552 1.02:1
ToolbarMinimalPerf.default 1045 1028 1.02:1
Avatar.Fluent 369 363 1.02:1
Button.Fluent 625 610 1.02:1
Icon.Fluent 718 704 1.02:1
Tooltip.Fluent 580 571 1.02:1
ButtonOverridesMissPerf.default 1767 1752 1.01:1
ChatWithPopoverPerf.default 471 468 1.01:1
DatepickerMinimalPerf.default 48261 47818 1.01:1
ListWith60ListItems.default 656 651 1.01:1
MenuButtonMinimalPerf.default 1669 1647 1.01:1
RadioGroupMinimalPerf.default 487 481 1.01:1
Dialog.Fluent 835 824 1.01:1
BoxMinimalPerf.default 411 412 1:1
ButtonSlotsPerf.default 621 623 1:1
ButtonUseCssPerf.default 876 874 1:1
ChatMinimalPerf.default 666 664 1:1
CheckboxMinimalPerf.default 2867 2878 1:1
DropdownMinimalPerf.default 3037 3035 1:1
HeaderMinimalPerf.default 434 432 1:1
LabelMinimalPerf.default 464 462 1:1
MenuMinimalPerf.default 941 944 1:1
ProviderMinimalPerf.default 986 983 1:1
SliderMinimalPerf.default 1604 1611 1:1
TableManyItemsPerf.default 2205 2203 1:1
CustomToolbarPrototype.default 3704 3709 1:1
TreeMinimalPerf.default 840 836 1:1
VideoMinimalPerf.default 707 706 1:1
Image.Fluent 448 448 1:1
TreeWith60ListItems.default 195 197 0.99:1
Checkbox.Fluent 648 652 0.99:1
AttachmentSlotsPerf.default 1243 1267 0.98:1
ChatDuplicateMessagesPerf.default 384 390 0.98:1
EmbedMinimalPerf.default 4331 4410 0.98:1
ProviderMergeThemesPerf.default 1546 1583 0.98:1
Dropdown.Fluent 3020 3096 0.98:1
ButtonUseCssNestingPerf.default 1093 1124 0.97:1
ListCommonPerf.default 710 735 0.97:1
ListNestedPerf.default 589 609 0.97:1
Slider.Fluent 1614 1671 0.97:1
RosterPerf.default 1236 1286 0.96:1

@ecraig12345 ecraig12345 merged commit 47b42ef into microsoft:master Feb 9, 2021
@ecraig12345 ecraig12345 deleted the no-react-internal branch February 9, 2021 10:21
@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@rockcs1992
Copy link
Contributor

@JustSlone It's going to collide with a lot of things, and there's not really any way around that. :(

That's fine. I just merged the latest master to my working branch. It did conflict with something but not that bad.

@ecraig12345 ecraig12345 mentioned this pull request Feb 10, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xugao xugao xugao left review comments

@khmakoto khmakoto khmakoto approved these changes

@JustSlone JustSlone JustSlone approved these changes

@aditima aditima Awaiting requested review from aditima

@behowell behowell Awaiting requested review from behowell behowell is a code owner

@dzearing dzearing Awaiting requested review from dzearing dzearing is a code owner

@evlevy evlevy Awaiting requested review from evlevy

@Jahnp Jahnp Awaiting requested review from Jahnp Jahnp is a code owner

@jdhuntington jdhuntington Awaiting requested review from jdhuntington

@joschect joschect Awaiting requested review from joschect

@joschemd joschemd Awaiting requested review from joschemd

@leddie24 leddie24 Awaiting requested review from leddie24

@lorejoh12 lorejoh12 Awaiting requested review from lorejoh12

@Markionium Markionium Awaiting requested review from Markionium

@micahgodbolt micahgodbolt Awaiting requested review from micahgodbolt

@mtennoe mtennoe Awaiting requested review from mtennoe

@ThomasMichon ThomasMichon Awaiting requested review from ThomasMichon ThomasMichon is a code owner

@Vitalius1 Vitalius1 Awaiting requested review from Vitalius1

@Raghurk Raghurk Awaiting requested review from Raghurk

Assignees

@joschect joschect

Labels
PR: API Modified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ecraig12345 @JustSlone @fabricteam @msft-fluent-ui-bot @rockcs1992 @xugao @khmakoto @joschect