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.

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

compose: adding mergeProps utility to react-compose #13360

Merged
merged 44 commits into from
Jun 1, 2020
Merged

compose: adding mergeProps utility to react-compose #13360

merged 44 commits into from
Jun 1, 2020

Conversation

dzearing
Copy link
Member

@dzearing dzearing commented May 27, 2020

This PR splits out more of my draft PR, and adds a mergeProps helper to the compose package. This helper will help resolve slots and slotProps and effectively can replace the resolveSlotProps that is nested within mergeComposeOptions. Expected usage:

const Button = compose((props, ref, options) => {
  const state = useButton(props); // optional
  const { slots, slotProps } = mergeProps(state /* or just props */, options);

  return <slots.root { ...slotProps.root } />;
}, {
  /* options */
});

I have not removed the existing implementation so that the v0 code is not busted, but once this is in, that code can be updated.

I am going through the shorthand factory tests right now to validate I didn't miss any, and will be adding more tests to this PR but wanted to get this going sooner for early feedback.

@dzearing dzearing changed the title compose: adding resolve callback to options to resolve conformance concerns compose: adding resolve callback to options May 27, 2020
@msft-github-bot
Copy link
Contributor

msft-github-bot commented May 27, 2020

Perf Analysis

Scenario Master Ticks PR Ticks Iterations Status
ButtonNext 445 487 5000 Possible regression
All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 853 860 5000
ButtonNext 445 487 5000 Possible regression
Checkbox 1702 1699 5000
CheckboxBase 1378 1352 5000
CheckboxNext 1634 1588 5000
ChoiceGroup 5297 5269 5000
ComboBox 950 956 1000
CommandBar 8274 8322 1000
ContextualMenu 14476 14565 1000
DefaultButton 1111 1125 5000
DetailsRow 3692 3668 5000
DetailsRow (fast icons) 3672 3730 5000
DetailsRow without styles 3443 3455 5000
Dialog 1559 1570 1000
DocumentCardTitle with truncation 1985 2071 1000
Dropdown 2555 2563 5000
FocusZone 1862 1850 5000
IconButton 1776 1822 5000
Label 320 338 5000
Link 478 487 5000
LinkNext 480 511 5000
MenuButton 1507 1486 5000
Nav 3388 3437 1000
Panel 1516 1504 1000
Persona 898 869 1000
Pivot 1455 1458 1000
PivotNext 831 812 5000
PrimaryButton 1305 1282 5000
SearchBox 1292 1316 5000
Slider 1512 1541 5000
SliderNext 2061 2041 5000
Spinner 417 426 5000
SplitButton 3269 3262 5000
Stack 497 504 5000
Stack with Intrinsic children 1962 1929 5000
Stack with Text children 5198 5162 5000
TagPicker 2916 2893 5000
Text 427 410 5000
TextField 1468 1446 5000
ThemeProvider 2833 2884 5000
Toggle 898 914 5000
ToggleNext 865 880 5000
button 87 72 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.72 0.52 1.38:1 2000 1442
🦄 Button.Fluent 0.11 0.2 0.55:1 5000 556
🔧 Checkbox.Fluent 1.16 0.37 3.14:1 1000 1158
🦄 Dialog.Fluent 0.15 0.22 0.68:1 5000 734
🔧 Dropdown.Fluent 6.72 0.47 14.3:1 1000 6724
🔧 Icon.Fluent 0.14 0.06 2.33:1 5000 712
🦄 Image.Fluent 0.07 0.11 0.64:1 5000 371
🔧 Slider.Fluent 3.02 0.38 7.95:1 1000 3019
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 377
🦄 Tooltip.Fluent 0.1 19.18 0.01:1 5000 519

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ChatDuplicateMessagesPerf.default 575 518 1.11:1
VideoMinimalPerf.default 651 591 1.1:1
LabelMinimalPerf.default 431 394 1.09:1
CardMinimalPerf.default 603 559 1.08:1
HeaderSlotsPerf.default 840 779 1.08:1
PopupMinimalPerf.default 296 276 1.07:1
AttachmentMinimalPerf.default 164 154 1.06:1
AvatarMinimalPerf.default 794 752 1.06:1
Checkbox.Fluent 1158 1090 1.06:1
DialogMinimalPerf.default 770 734 1.05:1
Text.Fluent 377 359 1.05:1
AccordionMinimalPerf.default 146 140 1.04:1
MenuMinimalPerf.default 875 844 1.04:1
RefMinimalPerf.default 220 211 1.04:1
EmbedMinimalPerf.default 3596 3506 1.03:1
FormMinimalPerf.default 415 401 1.03:1
ProviderMinimalPerf.default 892 868 1.03:1
StatusMinimalPerf.default 726 702 1.03:1
ChatWithPopoverPerf.default 555 545 1.02:1
CheckboxMinimalPerf.default 5482 5374 1.02:1
ImageMinimalPerf.default 379 373 1.02:1
ListNestedPerf.default 1139 1117 1.02:1
MenuButtonMinimalPerf.default 1812 1780 1.02:1
PortalMinimalPerf.default 129 127 1.02:1
Dropdown.Fluent 6724 6579 1.02:1
Icon.Fluent 712 698 1.02:1
Image.Fluent 371 364 1.02:1
Tooltip.Fluent 519 509 1.02:1
CarouselMinimalPerf.default 547 542 1.01:1
HeaderMinimalPerf.default 362 359 1.01:1
ItemLayoutMinimalPerf.default 2411 2389 1.01:1
ProviderMergeThemesPerf.default 2030 2013 1.01:1
SplitButtonMinimalPerf.default 4137 4104 1.01:1
TableManyItemsPerf.default 2511 2485 1.01:1
ToolbarMinimalPerf.default 968 958 1.01:1
AttachmentSlotsPerf.default 1288 1285 1:1
BoxMinimalPerf.default 336 337 1:1
DropdownMinimalPerf.default 6744 6723 1:1
TooltipMinimalPerf.default 767 765 1:1
AlertMinimalPerf.default 335 339 0.99:1
DropdownManyItemsPerf.default 2274 2291 0.99:1
GridMinimalPerf.default 1361 1377 0.99:1
ListMinimalPerf.default 476 481 0.99:1
ListWith60ListItems.default 1647 1664 0.99:1
ReactionMinimalPerf.default 392 394 0.99:1
IconMinimalPerf.default 703 707 0.99:1
TextMinimalPerf.default 354 357 0.99:1
CustomToolbarPrototype.default 5118 5168 0.99:1
TreeWith60ListItems.default 294 298 0.99:1
Button.Fluent 556 562 0.99:1
ButtonSlotsPerf.default 758 774 0.98:1
DividerMinimalPerf.default 349 357 0.98:1
InputMinimalPerf.default 1648 1676 0.98:1
SliderMinimalPerf.default 2987 3051 0.98:1
Avatar.Fluent 1442 1475 0.98:1
Dialog.Fluent 734 749 0.98:1
Slider.Fluent 3019 3085 0.98:1
ChatMinimalPerf.default 593 609 0.97:1
FlexMinimalPerf.default 304 314 0.97:1
RadioGroupMinimalPerf.default 408 419 0.97:1
TreeMinimalPerf.default 1349 1385 0.97:1
LayoutMinimalPerf.default 411 429 0.96:1
ButtonMinimalPerf.default 174 183 0.95:1
HierarchicalTreeMinimalPerf.default 413 437 0.95:1
ListCommonPerf.default 1167 1228 0.95:1
SegmentMinimalPerf.default 335 351 0.95:1
TextAreaMinimalPerf.default 455 479 0.95:1
AnimationMinimalPerf.default 731 781 0.94:1
TableMinimalPerf.default 387 411 0.94:1
LoaderMinimalPerf.default 1112 1190 0.93:1

@DustyTheBot
Copy link

DustyTheBot commented Jun 1, 2020

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS against b2a7905

@dzearing
Copy link
Member Author

dzearing commented Jun 1, 2020

I did some sleuthing with webpack bailouts to understand why the bundle size jumped so much.

  1. There is some side effect code in utilities, causing us to pull in merge-styles. I've adjusted this and it brough the min size down from 12 KB to 8 KB.
  2. If I remove the reference to getNativeElementProps, which is what's responsible for filtering out the props applicable to the native element, we bring the bundle down to 4.9 KB.

@dzearing
Copy link
Member Author

dzearing commented Jun 1, 2020

I believe this is ready to go in.

@dzearing dzearing merged commit d5ed2c3 into microsoft:master Jun 1, 2020
@msft-github-bot
Copy link
Contributor

🎉@fluentui/react-compose@v0.7.0 has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉@fluentui/react-button@v0.1.12 has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉@uifabric/utilities@v7.20.3 has been released which incorporates this pull request.:tada:

Handy links:

miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
* Updating typings.

* Fixing the template variable names, splitting out utils.ts into separate files, removing react-is usage which wasn't in the package.json, no need to pull an external dependency to check if we're a forwardref.

* Change files

* Missed one merge conflict.

* reducing changes

* Update @fluentui-react-compose-2020-05-27-08-46-47-feat-compose-cleanup.json

* pr feedback

* more

* Moving classname changes to separate branch

* cleanup.

* Change files

* pass build

* cleanup

* pr updates.

* updates

* Updates to tests.

* more tests, types, etc.

* More tests.

* Updating api

* Updates resolve to be mergeProps, removes from the compose options, deletes unused code.

* Rename

* Updates to rename mapPropsToSlotProps.

* Update @fluentui-react-compose-2020-05-27-12-54-51-feat-create-options-resolver.json

* Update packages/react-compose/src/mergeProps.ts

Co-authored-by: Makoto Morimoto <Humberto.Morimoto@microsoft.com>

* fix.

* fixes.

* Change files

* api updates.

* renames

* oi

* Update to package.json

* undo.

* Button updates to address screener issues; private copy of mergeProps is not accurate any more.

* Change files

* Reduce bundle size by 3k by removing side effect code.

* Change files

* screener fix.

Co-authored-by: Makoto Morimoto <Humberto.Morimoto@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants