refactor: simplify mergeprops for label by chpalac · Pull Request #19946 · microsoft/fluentui

chpalac · 2021-09-24T01:34:44Z

Pull request checklist

Addresses an existing issue: Fixes @fluentui/react-label: use simplified prop merging #19681
Include a change request file using $ yarn change

Description of changes

Use simple prop merging as described in #19678

Focus areas to test

(optional)

fabricteam · 2021-09-24T01:44:15Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`react-label` Label	`8.789 kB` `3.604 kB`	`9.108 kB` `3.729 kB`	`-319 B` `-125 B`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`164.716 kB` `46.962 kB`
`react-components` react-components: FluentProvider & webLightTheme	`35.77 kB` `11.405 kB`
`react-slider` Slider	`32.503 kB` `10.195 kB`
`react-switch` Switch	`24.36 kB` `7.858 kB`

_{🤖 This report was generated against f1f30260e73cd4dd503ae4b765aa2927536db7ca}

size-auditor · 2021-09-24T01:49:34Z

Asset size changes

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

Baseline commit: f1f30260e73cd4dd503ae4b765aa2927536db7ca (build)

fabricteam · 2021-09-24T02:11:48Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	745	722	5000
BaseButton	mount	764	770	5000
Breadcrumb	mount	2183	2246	1000
ButtonNext	mount	389	387	5000
Checkbox	mount	1258	1301	5000
CheckboxBase	mount	1086	1088	5000
ChoiceGroup	mount	3827	3866	5000
ComboBox	mount	814	843	1000
CommandBar	mount	8743	8920	1000
ContextualMenu	mount	5380	5230	1000
DefaultButton	mount	967	910	5000
DetailsRow	mount	3235	3172	5000
DetailsRowFast	mount	3142	3161	5000
DetailsRowNoStyles	mount	2990	2950	5000
Dialog	mount	2113	1965	1000
DocumentCardTitle	mount	120	135	1000
Dropdown	mount	2693	2699	5000
FluentProviderNext	mount	6269	6267	5000
FluentProviderWithTheme	mount	303	311	10
FluentProviderWithTheme	virtual-rerender	82	84	10
FluentProviderWithTheme	virtual-rerender-with-unmount	396	429	10
FocusTrapZone	mount	1539	1569	5000
FocusZone	mount	1540	1557	5000
IconButton	mount	1419	1488	5000
Label	mount	283	294	5000
Layer	mount	2545	2439	5000
Link	mount	394	402	5000
MakeStyles	mount	1552	1540	50000
MenuButton	mount	1221	1221	5000
MessageBar	mount	1730	1741	5000
Nav	mount	2596	2819	1000
OverflowSet	mount	955	857	5000
Panel	mount	1927	1986	1000
Persona	mount	723	704	1000
Pivot	mount	1203	1164	1000
PrimaryButton	mount	1062	1095	5000
Rating	mount	6394	6603	5000
SearchBox	mount	1055	1134	5000
Shimmer	mount	2027	2186	5000
Slider	mount	1654	1565	5000
SpinButton	mount	4216	4230	5000
Spinner	mount	322	368	5000
SplitButton	mount	2664	2683	5000
Stack	mount	366	423	5000
StackWithIntrinsicChildren	mount	1427	1399	5000
StackWithTextChildren	mount	3877	3819	5000
SwatchColorPicker	mount	8672	8482	5000
Tabs	mount	1204	1192	1000
TagPicker	mount	2219	2139	5000
TeachingBubble	mount	11214	10988	5000
Text	mount	357	359	5000
TextField	mount	1096	1143	5000
ThemeProvider	mount	992	907	5000
ThemeProvider	virtual-rerender	507	518	5000
ThemeProvider	virtual-rerender-with-unmount	1552	1483	5000
Toggle	mount	650	633	5000
buttonNative	mount	101	99	5000

Perf Analysis (`@fluentui/react-northstar`)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AvatarMinimalPerf.default	173	145	1.19:1
AlertMinimalPerf.default	234	203	1.15:1
AttachmentSlotsPerf.default	941	823	1.14:1
ReactionMinimalPerf.default	324	286	1.13:1
PortalMinimalPerf.default	153	141	1.09:1
ListCommonPerf.default	554	514	1.08:1
TextAreaMinimalPerf.default	438	405	1.08:1
MenuButtonMinimalPerf.default	1415	1321	1.07:1
BoxMinimalPerf.default	303	287	1.06:1
ChatMinimalPerf.default	563	531	1.06:1
DividerMinimalPerf.default	321	303	1.06:1
HeaderSlotsPerf.default	652	613	1.06:1
AnimationMinimalPerf.default	324	308	1.05:1
ButtonMinimalPerf.default	153	146	1.05:1
TooltipMinimalPerf.default	865	822	1.05:1
FlexMinimalPerf.default	254	245	1.04:1
ListWith60ListItems.default	570	546	1.04:1
PopupMinimalPerf.default	522	503	1.04:1
SegmentMinimalPerf.default	311	299	1.04:1
SkeletonMinimalPerf.default	307	296	1.04:1
TreeWith60ListItems.default	154	148	1.04:1
CardMinimalPerf.default	482	470	1.03:1
DropdownMinimalPerf.default	2736	2664	1.03:1
GridMinimalPerf.default	298	288	1.03:1
HeaderMinimalPerf.default	315	306	1.03:1
InputMinimalPerf.default	1141	1113	1.03:1
SliderMinimalPerf.default	1399	1357	1.03:1
ButtonSlotsPerf.default	479	469	1.02:1
ChatWithPopoverPerf.default	333	325	1.02:1
CheckboxMinimalPerf.default	2312	2272	1.02:1
FormMinimalPerf.default	353	346	1.02:1
LabelMinimalPerf.default	333	327	1.02:1
RefMinimalPerf.default	216	211	1.02:1
LoaderMinimalPerf.default	597	593	1.01:1
ProviderMinimalPerf.default	998	984	1.01:1
StatusMinimalPerf.default	580	574	1.01:1
TextMinimalPerf.default	303	301	1.01:1
CustomToolbarPrototype.default	3496	3477	1.01:1
TreeMinimalPerf.default	680	675	1.01:1
AccordionMinimalPerf.default	127	127	1:1
ListMinimalPerf.default	433	433	1:1
RadioGroupMinimalPerf.default	388	389	1:1
ToolbarMinimalPerf.default	792	792	1:1
ChatDuplicateMessagesPerf.default	258	260	0.99:1
LayoutMinimalPerf.default	312	316	0.99:1
ListNestedPerf.default	468	471	0.99:1
MenuMinimalPerf.default	708	715	0.99:1
VideoMinimalPerf.default	537	545	0.99:1
DatepickerMinimalPerf.default	4687	4760	0.98:1
DialogMinimalPerf.default	622	633	0.98:1
SplitButtonMinimalPerf.default	3512	3582	0.98:1
IconMinimalPerf.default	534	545	0.98:1
AttachmentMinimalPerf.default	129	135	0.96:1
ButtonOverridesMissPerf.default	1457	1521	0.96:1
CarouselMinimalPerf.default	363	378	0.96:1
EmbedMinimalPerf.default	3489	3644	0.96:1
TableManyItemsPerf.default	1510	1588	0.95:1
DropdownManyItemsPerf.default	547	579	0.94:1
TableMinimalPerf.default	322	341	0.94:1
ImageMinimalPerf.default	283	305	0.93:1
RosterPerf.default	971	1042	0.93:1
ProviderMergeThemesPerf.default	1363	1464	0.93:1
ItemLayoutMinimalPerf.default	916	1027	0.89:1

codesandbox-ci · 2021-09-24T06:03:42Z

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 5f4910e:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

…mple-mergeprops-label

packages/react-label/src/components/Label/useLabel.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

khmakoto

Approved with minor comments

khmakoto · 2021-09-24T06:27:17Z

packages/react-label/src/components/Label/useLabel.tsx

+import { getNativeElementProps } from '@fluentui/react-utilities';
+import type { LabelProps, LabelSlots, LabelState } from './Label.types';
+import { resolveShorthand } from '@fluentui/react-utilities';


nit:

Suggested change

import { getNativeElementProps } from '@fluentui/react-utilities';

import type { LabelProps, LabelSlots, LabelState } from './Label.types';

import { resolveShorthand } from '@fluentui/react-utilities';

import { getNativeElementProps, resolveShorthand } from '@fluentui/react-utilities';

import type { LabelProps, LabelSlots, LabelState } from './Label.types';

khmakoto · 2021-09-24T06:27:34Z

packages/react-label/src/components/Label/useLabel.tsx

-export const labelShorthandProps: LabelShorthandProps[] = ['required'];
-
-const mergeProps = makeMergeProps<LabelState>({ deepMerge: labelShorthandProps });
+export const labelShorthandProps: Array<keyof LabelSlots> = ['root', 'required'];


This can be inlined as it is being used in only one place now.

khmakoto · 2021-09-24T16:48:22Z

packages/react-checkbox/src/components/Checkbox/Checkbox.types.ts

  IntrinsicShorthandProps,
  ObjectShorthandProps,
 } from '@fluentui/react-utilities';
+import { LabelProps } from '@fluentui/react-label';


Suggested change

import { LabelProps } from '@fluentui/react-label';

import type { LabelProps } from '@fluentui/react-label';

khmakoto · 2021-09-24T16:50:45Z

packages/react-label/src/components/Label/useLabel.tsx

+    required: resolveShorthand(required === false ? null : required, {
+      required: !!required,
+      defaultProps: { children: '*' },
+    }),


nit: Move this to the end of this return statement so that all slots are processed next to each other.

* refactor: simplify mergeprops for label * Change files * add api changes * Updates Checkbox to use LabelProps on root * Updates root props * Update packages/react-label/src/components/Label/useLabel.tsx Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * Removes conditional blocks in favor of ternary operand * Updates checkbox API * Change files * Fix error on label size * Updates API Co-authored-by: Bernardo Sunderhus <bsunderhus@microsoft.com> Co-authored-by: Bernardo Sunderhus <bernardo.sunderhus@gmail.com> Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

refactor: simplify mergeprops for label

18705d7

chpalac requested a review from a team as a code owner September 24, 2021 01:34

msft-fluent-ui-bot added the Component: Label label Sep 24, 2021

msft-fluent-ui-bot assigned GeoffCoxMSFT Sep 24, 2021

Change files

624e479

add api changes

b85c889

chpalac and others added 3 commits September 24, 2021 04:37

Merge branch 'master' of github.com:microsoft/fluentui into refact/si…

b875197

…mple-mergeprops-label

Merge branch 'master' into pr/chassunc/19946

4779f7b

Updates Checkbox to use LabelProps on root

90661f4

layershifter reviewed Sep 24, 2021

View reviewed changes

packages/react-label/src/components/Label/useLabel.tsx Outdated Show resolved Hide resolved

packages/react-label/src/components/Label/useLabel.tsx Outdated Show resolved Hide resolved

Updates root props

0ba313a

chpalac requested a review from khmakoto as a code owner September 24, 2021 09:56

bsunderhus and others added 7 commits September 24, 2021 11:57

Update packages/react-label/src/components/Label/useLabel.tsx

ab3b91e

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Removes conditional blocks in favor of ternary operand

f270287

Merge branch 'master' into pr/chassunc/19946

8e41dc7

Updates checkbox API

fabecb6

Change files

90f311b

Fix error on label size

e2ad414

Updates API

5f4910e

bsunderhus enabled auto-merge (squash) September 24, 2021 12:46

khmakoto approved these changes Sep 24, 2021

View reviewed changes

bsunderhus merged commit 7c9c432 into microsoft:master Sep 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: simplify mergeprops for label#19946

refactor: simplify mergeprops for label#19946
bsunderhus merged 14 commits intomicrosoft:masterfrom
chpalac:refact/simple-mergeprops-label

chpalac commented Sep 24, 2021 •

edited

Loading

Uh oh!

fabricteam commented Sep 24, 2021 •

edited

Loading

Uh oh!

size-auditor bot commented Sep 24, 2021 •

edited

Loading

Uh oh!

fabricteam commented Sep 24, 2021 •

edited

Loading

Uh oh!

codesandbox-ci bot commented Sep 24, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

khmakoto left a comment

Uh oh!

khmakoto Sep 24, 2021

Uh oh!

khmakoto Sep 24, 2021

Uh oh!

khmakoto Sep 24, 2021

Uh oh!

khmakoto Sep 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

	import { LabelProps } from '@fluentui/react-label';
	import type { LabelProps } from '@fluentui/react-label';

Conversation

chpalac commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Focus areas to test

Uh oh!

fabricteam commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

size-auditor bot commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react)

Perf Analysis (@fluentui/react-northstar)

Uh oh!

codesandbox-ci bot commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

khmakoto left a comment

Choose a reason for hiding this comment

Uh oh!

khmakoto Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

khmakoto Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

khmakoto Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

khmakoto Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

chpalac commented Sep 24, 2021 •

edited

Loading

fabricteam commented Sep 24, 2021 •

edited

Loading

size-auditor bot commented Sep 24, 2021 •

edited

Loading

fabricteam commented Sep 24, 2021 •

edited

Loading

Perf Analysis (`@fluentui/react`)

Perf Analysis (`@fluentui/react-northstar`)

codesandbox-ci bot commented Sep 24, 2021 •

edited

Loading