Skip to content

ThemeProvider: Fixing RecursivePartial #14562

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
Merged
dzearing merged 6 commits into from
Aug 18, 2020
Merged

ThemeProvider: Fixing RecursivePartial #14562

dzearing merged 6 commits into from
Aug 18, 2020

Conversation

@dzearing
Copy link
Member

@dzearing dzearing commented Aug 17, 2020

The typings for RecursivePartial were not working correctly, and subsequently typing errors would occur from partial themes containing tokens without body. This should resolve the issue.

@dzearing dzearing mentioned this pull request Aug 17, 2020
Merged
2 tasks
@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 17, 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 6149247:

Sandbox Source
Fluent UI Button Configuration

xugao
xugao reviewed Aug 17, 2020
View reviewed changes
xugao
xugao approved these changes Aug 17, 2020
View reviewed changes
Copy link
Contributor

@xugao xugao 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 fixing!

@msft-github-bot
Copy link
Contributor

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

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 881 872 5000
ButtonNext mount 566 579 5000
Checkbox mount 1516 1498 5000
CheckboxBase mount 1232 1301 5000
CheckboxNext mount 1681 1671 5000
ChoiceGroup mount 4846 4955 5000
ComboBox mount 885 946 1000
CommandBar mount 7547 7389 1000
ContextualMenu mount 13724 13850 1000
DefaultButton mount 1103 1106 5000
DetailsRow mount 3566 3512 5000
DetailsRowFast mount 3471 3571 5000
DetailsRowNoStyles mount 3349 3346 5000
Dialog mount 1518 1500 1000
DocumentCardTitle mount 1800 1784 1000
Dropdown mount 2635 2609 5000
FocusZone mount 1800 1784 5000
IconButton mount 1725 1707 5000
Label mount 340 334 5000
Link mount 460 438 5000
LinkNext mount 480 471 5000
MenuButton mount 1448 1417 5000
Nav mount 3191 3203 1000
Panel mount 1427 1425 1000
Persona mount 831 823 1000
Pivot mount 1419 1366 1000
PivotNext mount 1376 1386 1000
PrimaryButton mount 1234 1220 5000
SearchBox mount 1242 1290 5000
SearchBoxNext mount 1308 1290 5000
Slider mount 1504 1529 5000
SliderNext mount 1893 1899 5000
SpinButton mount 4864 4927 5000
SpinButtonNext mount 5077 5090 5000
Spinner mount 437 411 5000
SplitButton mount 3136 3124 5000
Stack mount 508 535 5000
StackWithIntrinsicChildren mount 1897 1928 5000
StackWithTextChildren mount 4924 4910 5000
TagPicker mount 2748 2690 5000
Text mount 427 431 5000
TextField mount 1292 1369 5000
ThemeProvider mount 2942 2808 5000
ThemeProvider virtual-rerender 456 442 5000
Toggle mount 845 827 5000
ToggleNext mount 840 830 5000
button mount 124 123 5000

Perf Analysis (Fluent)

⚠️ 3 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonOverridesMissPerf.default 116 42 2.76:1 analysis
ButtonUseCssNestingPerf.default 60 43 1.4:1 analysis
ListNestedPerf.default 573 860 0.67:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.42 0.47 0.89:1 2000 837
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 528
🔧 Checkbox.Fluent 0.64 0.34 1.88:1 1000 644
🎯 Dialog.Fluent 0.16 0.23 0.7:1 5000 780
🔧 Dropdown.Fluent 2.99 0.47 6.36:1 1000 2985
🔧 Icon.Fluent 0.14 0.05 2.8:1 5000 688
🦄 Image.Fluent 0.07 0.11 0.64:1 5000 354
🔧 Slider.Fluent 1.59 0.34 4.68:1 1000 1588
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 341
🦄 Tooltip.Fluent 0.11 16.85 0.01:1 5000 542

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonUseCssPerf.default 56 43 1.3:1
AttachmentMinimalPerf.default 170 144 1.18:1
PortalMinimalPerf.default 132 113 1.17:1
IconMinimalPerf.default 672 574 1.17:1
AccordionMinimalPerf.default 156 142 1.1:1
RefMinimalPerf.default 216 197 1.1:1
ChatWithPopoverPerf.default 519 476 1.09:1
SkeletonMinimalPerf.default 401 370 1.08:1
Text.Fluent 341 317 1.08:1
BoxMinimalPerf.default 342 321 1.07:1
FlexMinimalPerf.default 283 264 1.07:1
ButtonSlotsPerf.default 606 577 1.05:1
CardMinimalPerf.default 548 522 1.05:1
LoaderMinimalPerf.default 766 728 1.05:1
ProviderMergeThemesPerf.default 1973 1887 1.05:1
ProviderMinimalPerf.default 965 916 1.05:1
RadioGroupMinimalPerf.default 405 386 1.05:1
StatusMinimalPerf.default 672 640 1.05:1
TreeMinimalPerf.default 853 810 1.05:1
CustomToolbarPrototype.default 3836 3701 1.04:1
VideoMinimalPerf.default 601 578 1.04:1
Dialog.Fluent 780 749 1.04:1
CarouselMinimalPerf.default 449 438 1.03:1
DividerMinimalPerf.default 339 328 1.03:1
GridMinimalPerf.default 321 313 1.03:1
ItemLayoutMinimalPerf.default 1240 1206 1.03:1
LabelMinimalPerf.default 390 378 1.03:1
TableMinimalPerf.default 393 383 1.03:1
TextMinimalPerf.default 333 323 1.03:1
ToolbarMinimalPerf.default 904 878 1.03:1
Dropdown.Fluent 2985 2906 1.03:1
Tooltip.Fluent 542 524 1.03:1
AttachmentSlotsPerf.default 1144 1123 1.02:1
DialogMinimalPerf.default 755 737 1.02:1
FormMinimalPerf.default 382 373 1.02:1
HeaderSlotsPerf.default 757 741 1.02:1
LayoutMinimalPerf.default 377 368 1.02:1
MenuButtonMinimalPerf.default 1536 1502 1.02:1
ReactionMinimalPerf.default 373 364 1.02:1
TooltipMinimalPerf.default 761 745 1.02:1
ButtonMinimalPerf.default 172 170 1.01:1
CheckboxMinimalPerf.default 2887 2854 1.01:1
EmbedMinimalPerf.default 1889 1878 1.01:1
ImageMinimalPerf.default 354 350 1.01:1
MenuMinimalPerf.default 837 825 1.01:1
PopupMinimalPerf.default 665 659 1.01:1
TableManyItemsPerf.default 2149 2125 1.01:1
Icon.Fluent 688 683 1.01:1
ChatMinimalPerf.default 597 595 1:1
DropdownManyItemsPerf.default 716 718 1:1
HeaderMinimalPerf.default 339 339 1:1
InputMinimalPerf.default 1298 1292 1:1
ListMinimalPerf.default 442 443 1:1
SegmentMinimalPerf.default 326 327 1:1
SplitButtonMinimalPerf.default 3697 3711 1:1
Checkbox.Fluent 644 644 1:1
Image.Fluent 354 355 1:1
AvatarMinimalPerf.default 463 466 0.99:1
DropdownMinimalPerf.default 2914 2932 0.99:1
TextAreaMinimalPerf.default 443 446 0.99:1
TreeWith60ListItems.default 208 211 0.99:1
Button.Fluent 528 533 0.99:1
AnimationMinimalPerf.default 378 385 0.98:1
Slider.Fluent 1588 1614 0.98:1
AlertMinimalPerf.default 292 300 0.97:1
ChatDuplicateMessagesPerf.default 410 423 0.97:1
Avatar.Fluent 837 860 0.97:1
SliderMinimalPerf.default 1550 1611 0.96:1
ListWith60ListItems.default 942 1084 0.87:1
ListCommonPerf.default 664 912 0.73:1

@size-auditor
Copy link

size-auditor bot commented Aug 18, 2020

Asset size changes

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

Baseline commit: 419049fdb8b6acd4f9525a3c7cf2da1c41569612 (build)

@dzearing dzearing merged commit f155a92 into microsoft:master Aug 18, 2020
@dzearing dzearing deleted the fix/recursive-partial branch August 18, 2020 00:50
@msft-github-bot
Copy link
Contributor

msft-github-bot commented Aug 18, 2020

🎉@fluentui/react-theme-provider@v0.7.3 has been released which incorporates this pull request.:tada:

Handy links:

levithomason pushed a commit to levithomason/fluentui that referenced this pull request Aug 24, 2020
@dzearing @levithomason
ThemeProvider: Fixing RecursivePartial (microsoft#14562)
8ac99a4 
* Fixing recursive partial.

* Change files

* whitespace

* updates

* Updates.

* Change files
@msft-github-bot
Copy link
Contributor

msft-github-bot commented Oct 6, 2020

🎉@fluentui/react-theme-provider@v0.14.4 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@xugao xugao xugao approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@khmakoto khmakoto

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dzearing @msft-github-bot @xugao @khmakoto