react-text - Simplified prop merging and root as a slot#19711
react-text - Simplified prop merging and root as a slot#19711theerebuss merged 31 commits intomicrosoft:masterfrom theerebuss:text-simplified-prop
Conversation
📊 Bundle size reportUnchanged fixtures
|
|
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 54cb741:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: c6c1ae1a69d9a2faa4db5eebf981493a2154e1f0 (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 976 | 1008 | 5000 | |
| BaseButton | mount | 1009 | 1010 | 5000 | |
| Breadcrumb | mount | 2807 | 2724 | 1000 | |
| ButtonNext | mount | 470 | 479 | 5000 | |
| Checkbox | mount | 1656 | 1692 | 5000 | |
| CheckboxBase | mount | 1463 | 1461 | 5000 | |
| ChoiceGroup | mount | 5185 | 5208 | 5000 | |
| ComboBox | mount | 1062 | 1070 | 1000 | |
| CommandBar | mount | 10812 | 10754 | 1000 | |
| ContextualMenu | mount | 6927 | 6949 | 1000 | |
| DefaultButton | mount | 1243 | 1260 | 5000 | |
| DetailsRow | mount | 4093 | 4031 | 5000 | |
| DetailsRowFast | mount | 4085 | 4220 | 5000 | |
| DetailsRowNoStyles | mount | 3812 | 3918 | 5000 | |
| Dialog | mount | 2521 | 2551 | 1000 | |
| DocumentCardTitle | mount | 167 | 161 | 1000 | |
| Dropdown | mount | 3548 | 3516 | 5000 | |
| FluentProviderNext | mount | 7402 | 7421 | 5000 | |
| FluentProviderWithTheme | mount | 350 | 341 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 108 | 112 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 504 | 489 | 10 | |
| FocusTrapZone | mount | 1897 | 1870 | 5000 | |
| FocusZone | mount | 1899 | 1842 | 5000 | |
| IconButton | mount | 1910 | 1889 | 5000 | |
| Label | mount | 356 | 351 | 5000 | |
| Layer | mount | 3170 | 3269 | 5000 | |
| Link | mount | 514 | 509 | 5000 | |
| MakeStyles | mount | 1982 | 1949 | 50000 | |
| MenuButton | mount | 1626 | 1636 | 5000 | |
| MessageBar | mount | 2092 | 2104 | 5000 | |
| Nav | mount | 3580 | 3644 | 1000 | |
| OverflowSet | mount | 1201 | 1219 | 5000 | |
| Panel | mount | 2558 | 2505 | 1000 | |
| Persona | mount | 934 | 897 | 1000 | |
| Pivot | mount | 1560 | 1579 | 1000 | |
| PrimaryButton | mount | 1429 | 1394 | 5000 | |
| Rating | mount | 8540 | 8590 | 5000 | |
| SearchBox | mount | 1481 | 1445 | 5000 | |
| Shimmer | mount | 2877 | 2840 | 5000 | |
| Slider | mount | 2145 | 2135 | 5000 | |
| SpinButton | mount | 5428 | 5418 | 5000 | |
| Spinner | mount | 453 | 445 | 5000 | |
| SplitButton | mount | 3395 | 3506 | 5000 | |
| Stack | mount | 549 | 532 | 5000 | |
| StackWithIntrinsicChildren | mount | 1722 | 1755 | 5000 | |
| StackWithTextChildren | mount | 5220 | 5225 | 5000 | |
| SwatchColorPicker | mount | 11674 | 11009 | 5000 | |
| Tabs | mount | 1561 | 1590 | 1000 | |
| TagPicker | mount | 2970 | 3025 | 5000 | |
| TeachingBubble | mount | 14303 | 14300 | 5000 | |
| Text | mount | 482 | 484 | 5000 | |
| TextField | mount | 1567 | 1543 | 5000 | |
| ThemeProvider | mount | 1261 | 1260 | 5000 | |
| ThemeProvider | virtual-rerender | 647 | 644 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2100 | 2060 | 5000 | |
| Toggle | mount | 892 | 906 | 5000 | |
| buttonNative | mount | 126 | 134 | 5000 |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| TreeWith60ListItems.default | 210 | 184 | 1.14:1 |
| ButtonMinimalPerf.default | 217 | 198 | 1.1:1 |
| HeaderMinimalPerf.default | 424 | 384 | 1.1:1 |
| StatusMinimalPerf.default | 809 | 736 | 1.1:1 |
| TextMinimalPerf.default | 398 | 362 | 1.1:1 |
| PortalMinimalPerf.default | 185 | 170 | 1.09:1 |
| AccordionMinimalPerf.default | 179 | 165 | 1.08:1 |
| AnimationMinimalPerf.default | 463 | 430 | 1.08:1 |
| ChatDuplicateMessagesPerf.default | 328 | 303 | 1.08:1 |
| FlexMinimalPerf.default | 333 | 307 | 1.08:1 |
| DividerMinimalPerf.default | 424 | 395 | 1.07:1 |
| LayoutMinimalPerf.default | 423 | 396 | 1.07:1 |
| AttachmentMinimalPerf.default | 190 | 179 | 1.06:1 |
| ReactionMinimalPerf.default | 441 | 418 | 1.06:1 |
| ToolbarMinimalPerf.default | 1118 | 1051 | 1.06:1 |
| BoxMinimalPerf.default | 410 | 389 | 1.05:1 |
| HeaderSlotsPerf.default | 862 | 820 | 1.05:1 |
| ListNestedPerf.default | 644 | 611 | 1.05:1 |
| RadioGroupMinimalPerf.default | 504 | 482 | 1.05:1 |
| IconMinimalPerf.default | 703 | 668 | 1.05:1 |
| TableManyItemsPerf.default | 2187 | 2092 | 1.05:1 |
| TableMinimalPerf.default | 444 | 421 | 1.05:1 |
| InputMinimalPerf.default | 1395 | 1337 | 1.04:1 |
| LoaderMinimalPerf.default | 762 | 736 | 1.04:1 |
| ProviderMergeThemesPerf.default | 1778 | 1705 | 1.04:1 |
| SegmentMinimalPerf.default | 421 | 404 | 1.04:1 |
| ButtonOverridesMissPerf.default | 1906 | 1845 | 1.03:1 |
| DropdownMinimalPerf.default | 3432 | 3318 | 1.03:1 |
| FormMinimalPerf.default | 489 | 474 | 1.03:1 |
| RosterPerf.default | 1359 | 1322 | 1.03:1 |
| SkeletonMinimalPerf.default | 397 | 386 | 1.03:1 |
| SliderMinimalPerf.default | 1689 | 1634 | 1.03:1 |
| TreeMinimalPerf.default | 908 | 880 | 1.03:1 |
| ChatMinimalPerf.default | 726 | 715 | 1.02:1 |
| EmbedMinimalPerf.default | 4531 | 4434 | 1.02:1 |
| LabelMinimalPerf.default | 443 | 434 | 1.02:1 |
| ListMinimalPerf.default | 581 | 567 | 1.02:1 |
| MenuButtonMinimalPerf.default | 1902 | 1867 | 1.02:1 |
| PopupMinimalPerf.default | 635 | 623 | 1.02:1 |
| TooltipMinimalPerf.default | 1144 | 1126 | 1.02:1 |
| DropdownManyItemsPerf.default | 787 | 777 | 1.01:1 |
| GridMinimalPerf.default | 383 | 380 | 1.01:1 |
| ListCommonPerf.default | 704 | 699 | 1.01:1 |
| MenuMinimalPerf.default | 939 | 929 | 1.01:1 |
| ProviderMinimalPerf.default | 1081 | 1066 | 1.01:1 |
| RefMinimalPerf.default | 251 | 249 | 1.01:1 |
| CardMinimalPerf.default | 628 | 625 | 1:1 |
| ChatWithPopoverPerf.default | 383 | 382 | 1:1 |
| ListWith60ListItems.default | 704 | 702 | 1:1 |
| VideoMinimalPerf.default | 715 | 712 | 1:1 |
| AttachmentSlotsPerf.default | 1223 | 1240 | 0.99:1 |
| CarouselMinimalPerf.default | 514 | 519 | 0.99:1 |
| CheckboxMinimalPerf.default | 2916 | 2937 | 0.99:1 |
| DatepickerMinimalPerf.default | 5867 | 5925 | 0.99:1 |
| ItemLayoutMinimalPerf.default | 1390 | 1409 | 0.99:1 |
| SplitButtonMinimalPerf.default | 4543 | 4588 | 0.99:1 |
| CustomToolbarPrototype.default | 4025 | 4065 | 0.99:1 |
| ButtonSlotsPerf.default | 596 | 611 | 0.98:1 |
| DialogMinimalPerf.default | 810 | 829 | 0.98:1 |
| ImageMinimalPerf.default | 416 | 441 | 0.94:1 |
| TextAreaMinimalPerf.default | 538 | 572 | 0.94:1 |
| AvatarMinimalPerf.default | 227 | 244 | 0.93:1 |
| AlertMinimalPerf.default | 294 | 325 | 0.9:1 |
| align: 'start', | ||
|
|
||
| return state; | ||
| components: { root: asProp }, |
There was a problem hiding this comment.
| components: { root: asProp }, | |
| components: { root: 'span' }, |
This configures slot's default behavior, should be hardcoded
state.components should have hardcoded values with matching element
#18844
There was a problem hiding this comment.
How do we allow the user to override the default rendered element?
There was a problem hiding this comment.
In this case:
- via
asprop, it should be handled viagetSlots(), i.e.<Text as="h1" />still should work - via composing new component:
function TextAsH1() {
const state = useText()
state.components.root = 'h1'
} Is there anything that breaks these scenarios?
There was a problem hiding this comment.
From what I'm seeing getSlot is picking the as prop from the slot in the state to get the element but getNativeElementProps strips as out as it's not a native prop. Should I not be using getNativeElementProps?
There was a problem hiding this comment.
Then let's keep it as it is for now. Can you please create an issue to track it? 🙏 It's not expected behavior
|
CI fails because you have outdated file from API extractor, please run |
| components: { root: 'span' }, | ||
|
|
||
| return state; | ||
| ...props, |
There was a problem hiding this comment.
I'm not sure if this needs to be here? I believe that ...props just goes on the root slot.
There was a problem hiding this comment.
| ...props, |
@czearing yes, it's correct. Overlooked it 🙄
There was a problem hiding this comment.
I seem to be missing something then. How will useTextStyles hook have access to the props then?
There was a problem hiding this comment.
From what I know, the props that are used for styling and what not are deconstructed and passed on:
const { value, defaultValue } = props;
const state: ComponentState = {
value,
defaultValue,
root: getNativeElementProps('div', {
...
}),Everything else gets passed to the root:
root: getNativeElementProps('span', {
ref,
...props,
id: useId('componentName-', props.id),
}),Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
…i into text-simplified-prop
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| */ | ||
| ref: React.Ref<HTMLElement>; | ||
| } | ||
| export interface TextState extends ComponentState<TextSlots>, TextCommons {} |
There was a problem hiding this comment.
Good point, I've gone ahead and migrated all interfaces to types.
Pull request checklist
$ yarn changeDescription of changes
react-text.