Update V9 ComponentCommons type to ComponentCommonsUnstable #21607

micahgodbolt · 2022-02-03T22:58:43Z

We plan to deprecate and get rid of the commons type, and while the commons type is not exported from react-components, it is exported from the individual component. Following the same 'unstable' function approach we took for some of our unstable functions, this change informs downstream users that this type could change in the future.

Could also change this to XCommonsDeprecated too, but this works fine and is consistent.

codesandbox-ci · 2022-02-03T23:04:51Z

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 0b8c83e:

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

fabricteam · 2022-02-03T23:05:20Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-accordion` Accordion (including children components)	`54.905 kB` `17.567 kB`
`react-avatar` Avatar	`43.36 kB` `12.495 kB`
`react-badge` Badge	`20.831 kB` `6.533 kB`
`react-badge` CounterBadge	`21.699 kB` `6.827 kB`
`react-badge` PresenceBadge	`21.785 kB` `6.507 kB`
`react-button` Button	`27.932 kB` `8.025 kB`
`react-button` CompoundButton	`33.196 kB` `8.987 kB`
`react-button` MenuButton	`29.584 kB` `8.598 kB`
`react-button` SplitButton	`35.729 kB` `9.676 kB`
`react-button` ToggleButton	`37.209 kB` `8.627 kB`
`react-card` Card - All	`48.512 kB` `14.352 kB`
`react-card` Card	`43.987 kB` `13.188 kB`
`react-card` CardFooter	`7.615 kB` `3.23 kB`
`react-card` CardHeader	`8.893 kB` `3.675 kB`
`react-card` CardPreview	`7.806 kB` `3.357 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`165.717 kB` `46.859 kB`
`react-components` react-components: FluentProvider & webLightTheme	`32.479 kB` `10.625 kB`
`react-divider` Divider	`15.256 kB` `5.511 kB`
`react-image` Image	`10.067 kB` `3.934 kB`
`react-label` Label	`8.303 kB` `3.472 kB`
`react-link` Link	`11.064 kB` `4.487 kB`
`react-menu` Menu (including children components)	`102.89 kB` `31.719 kB`
`react-menu` Menu (including selectable components)	`105.245 kB` `32.089 kB`
`react-popover` Popover	`95.816 kB` `29.246 kB`
`react-portal` Portal	`6.249 kB` `2.163 kB`
`react-provider` FluentProvider	`13.962 kB` `5.231 kB`
`react-slider` Slider	`22.928 kB` `7.747 kB`
`react-spinner` Spinner	`6.773 kB` `2.879 kB`
`react-switch` Switch	`25.387 kB` `8.204 kB`
`react-text` Text - Default	`10.755 kB` `4.212 kB`
`react-text` Text - Wrappers	`14.067 kB` `4.558 kB`
`react-tooltip` Tooltip	`42.455 kB` `14.61 kB`

_{🤖 This report was generated against c0d4e63ee58e60e2c6674efbacc0783cd520984e}

size-auditor · 2022-02-03T23:12:21Z

Asset size changes

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

Baseline commit: c0d4e63ee58e60e2c6674efbacc0783cd520984e (build)

fabricteam · 2022-02-03T23:31:34Z

Perf Analysis (`@fluentui/react`)

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
FluentProviderWithTheme	virtual-rerender	111	102	10	Possible regression

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
Avatar	mount	709	715	5000
BaseButton	mount	795	804	5000
Breadcrumb	mount	2265	2286	1000
ButtonNext	mount	409	418	5000
Checkbox	mount	1342	1338	5000
CheckboxBase	mount	1121	1119	5000
ChoiceGroup	mount	4131	4121	5000
ComboBox	mount	865	856	1000
CommandBar	mount	8794	8818	1000
ContextualMenu	mount	7365	7327	1000
DefaultButton	mount	1003	1015	5000
DetailsRow	mount	3248	3267	5000
DetailsRowFast	mount	3268	3212	5000
DetailsRowNoStyles	mount	3088	3062	5000
Dialog	mount	2242	2241	1000
DocumentCardTitle	mount	151	169	1000
Dropdown	mount	2831	2831	5000
FluentProviderNext	mount	1687	1762	5000
FluentProviderWithTheme	mount	170	161	10
FluentProviderWithTheme	virtual-rerender	111	102	10	Possible regression
FluentProviderWithTheme	virtual-rerender-with-unmount	169	177	10
FocusTrapZone	mount	1607	1598	5000
FocusZone	mount	1528	1563	5000
IconButton	mount	1535	1519	5000
Label	mount	333	328	5000
Layer	mount	2573	2610	5000
Link	mount	429	441	5000
MakeStyles	mount	1492	1514	50000
MenuButton	mount	1293	1269	5000
MessageBar	mount	1756	1730	5000
Nav	mount	2874	2891	1000
OverflowSet	mount	999	986	5000
Panel	mount	2183	2176	1000
Persona	mount	761	750	1000
Pivot	mount	1245	1281	1000
PrimaryButton	mount	1139	1142	5000
Rating	mount	6645	6605	5000
SearchBox	mount	1172	1164	5000
Shimmer	mount	2235	2205	5000
Slider	mount	1731	1689	5000
SpinButton	mount	4349	4344	5000
Spinner	mount	394	388	5000
SplitButton	mount	2722	2731	5000
Stack	mount	471	475	5000
StackWithIntrinsicChildren	mount	2006	2001	5000
StackWithTextChildren	mount	4511	4518	5000
SwatchColorPicker	mount	9924	9848	5000
TagPicker	mount	2273	2270	5000
TeachingBubble	mount	11390	11343	5000
Text	mount	403	387	5000
TextField	mount	1227	1247	5000
ThemeProvider	mount	1055	1038	5000
ThemeProvider	virtual-rerender	554	555	5000
ThemeProvider	virtual-rerender-with-unmount	1614	1615	5000
Toggle	mount	710	713	5000
buttonNative	mount	127	134	5000

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

⚠️ No perf measurements available

Hotell · 2022-02-07T14:45:09Z

so what was already done for react-components suite is to get rid of those type as part of public API surface. The general agreement was that we dont wanna expose Commons no matter how the RFC will evolve (merge/not-merge).

Based on that, I'd suggest to be consistent with our patterns/public apis thus rather then rename, I'd remove all the exports from packages and enable our lint rule as was done #21195.

side note:

using unstable suffix for types and real implementation is bit of a difference. While you cannot access the implementation function directly if it's not being exported ( that's why the unstable suffix makes sense ), you can always access the type information as it's included in Props/State anyways

micahgodbolt · 2022-02-07T23:22:37Z

Talked to @GeoffCox about our options here, and we're leaning towards the rename for a couple reasons:

it gives visibility to the underlined API within the api.md files. Changes made to the Commons type should still be tracked in API extractor.
Some components (like Button) rely on the type being exported

@JustSlone is going to confirm with LT in the morning before making any final decisions here.

Hotell · 2022-02-08T10:48:40Z

it gives visibility to the underlined API within the api.md files. Changes made to the Commons type should still be tracked in API extractor.

💡 To provide better context for reviewers, this is current output of api-extractor for *Commons within react-button

// @file etc/react-button.api.md

// @public (undocumented)
export type ButtonCommons = {
    appearance?: 'primary' | 'outline' | 'subtle' | 'transparent';
    block: boolean;
    disabledFocusable: boolean;
    disabled: boolean;
    iconPosition?: 'before' | 'after';
    shape: 'rounded' | 'circular' | 'square';
    size: 'small' | 'medium' | 'large';
};

// @public (undocumented)
export type ButtonProps = ComponentProps<ButtonSlots> & Partial<ButtonCommons>;

if we don't export *Common type from underlying package we'll get following:

// Warning: (ae-forgotten-export) The symbol "ButtonCommons" needs to be exported by the entry point index.d.ts
//
// @public (undocumented)
export type ButtonProps = ComponentProps<ButtonSlots> & Partial<ButtonCommons>;

This output might be an issue indeed. To better answer that, we should answer following questions:

which consumer do rely on the provided api.md and how does it affect them
how are we processing changes in the api.md (besides failing build if generated output doesn't match implementation)
react-components as a main source of truth for vNext doesn't exposes underlying packages API details at all besides exported tokens. How is this affecting consumers ?

Last but not least, our codebase contains inconsistencies already regarding how we approach exporting types.

Following packages contain Warning: (ae-forgotten-export) violation:

react-menu
react-positioning
react-utilities
react-text
style-utilities (v8)
web-components

Some components (like Button) rely on the type being exported

Not sure I follow. I guess you're referring to using ButtonCommons across all button-* components?

If that's the case, it is ok as the import statement uses deep import anyways which doesn't affects public API, thus this is not an issue

micahgodbolt · 2022-02-08T16:24:36Z

@Hotell ButtonCommons is used in other button type files

fluentui/packages/react-button/src/components/CompoundButton/CompoundButton.types.ts

Lines 1 to 19 in 9ba6686

    
           import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities'; 
        
           import type { ButtonCommons, ButtonSlots, ButtonState } from '../Button/Button.types'; 
        
           export type CompoundButtonSlots = ButtonSlots & { 
        
             /** 
        
              * Second line of text that describes the action this button takes. 
        
              */ 
        
             secondaryContent?: Slot<'span'>; 
        
             /** 
        
              * Container that wraps the children and the secondaryContent slot. 
        
              */ 
        
             contentContainer: NonNullable<Slot<'span'>>; 
        
           }; 
        
           export type CompoundButtonProps = ComponentProps<Partial<CompoundButtonSlots>> & Partial<ButtonCommons>; 
        
           export type CompoundButtonState = ComponentState<CompoundButtonSlots> & 
        
             Omit<ButtonState, keyof ButtonSlots | 'components'>;

micahgodbolt · 2022-02-08T21:28:17Z

Closing this PR down and going with the approach in #21660, leaving the type name unchanged, but not exporting them from the top level.

micahgodbolt added 2 commits February 3, 2022 14:47

change commons type to be unstable

38c55d2

new api files

4466802

micahgodbolt requested review from ling1726 and GeoffCoxMSFT February 3, 2022 22:58

micahgodbolt requested review from behowell, dzearing, ecraig12345, khmakoto, tomi-msft and a team as code owners February 3, 2022 22:58

msft-fluent-ui-bot assigned TristanWatanabe Feb 3, 2022

Change files

d443ecb

smhigley approved these changes Feb 3, 2022

View reviewed changes

upate dialog api

2fd2ffa

micahgodbolt closed this Feb 4, 2022

micahgodbolt reopened this Feb 4, 2022

micahgodbolt added 6 commits February 4, 2022 08:32

spin api

0aff87e

all the apis

b33e775

api for spinner

5d541e5

add radio

3eb3cf4

tabs

f1949d9

change story using commons

51c45b9

micahgodbolt enabled auto-merge (squash) February 4, 2022 17:54

khmakoto approved these changes Feb 4, 2022

View reviewed changes

GeoffCoxMSFT approved these changes Feb 4, 2022

View reviewed changes

layershifter approved these changes Feb 7, 2022

View reviewed changes

andrefcdias self-requested a review February 7, 2022 14:32

andrefcdias removed their request for review February 7, 2022 14:49

micahgodbolt disabled auto-merge February 7, 2022 15:33

micahgodbolt added 2 commits February 7, 2022 07:38

merge conflicts

1dfd070

tabs api update

0b8c83e

micahgodbolt enabled auto-merge (squash) February 7, 2022 17:09

micahgodbolt mentioned this pull request Feb 8, 2022

Remove commons export at component root #21660

Merged

micahgodbolt closed this Feb 8, 2022

auto-merge was automatically disabled February 8, 2022 21:28
Pull request was closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update V9 ComponentCommons type to ComponentCommonsUnstable #21607

Update V9 ComponentCommons type to ComponentCommonsUnstable #21607

micahgodbolt commented Feb 3, 2022

codesandbox-ci bot commented Feb 3, 2022 •

edited

fabricteam commented Feb 3, 2022 •

edited

size-auditor bot commented Feb 3, 2022 •

edited

fabricteam commented Feb 3, 2022 •

edited

Hotell commented Feb 7, 2022 •

edited

micahgodbolt commented Feb 7, 2022 •

edited

Hotell commented Feb 8, 2022

micahgodbolt commented Feb 8, 2022 •

edited

micahgodbolt commented Feb 8, 2022 •

edited

Update V9 ComponentCommons type to ComponentCommonsUnstable #21607

Update V9 ComponentCommons type to ComponentCommonsUnstable #21607

Conversation

micahgodbolt commented Feb 3, 2022

codesandbox-ci bot commented Feb 3, 2022 • edited

fabricteam commented Feb 3, 2022 • edited

📊 Bundle size report

size-auditor bot commented Feb 3, 2022 • edited

Asset size changes

fabricteam commented Feb 3, 2022 • edited

Perf Analysis (@fluentui/react)

Perf Analysis (@fluentui/react-northstar)

Hotell commented Feb 7, 2022 • edited

micahgodbolt commented Feb 7, 2022 • edited

Hotell commented Feb 8, 2022

micahgodbolt commented Feb 8, 2022 • edited

micahgodbolt commented Feb 8, 2022 • edited

codesandbox-ci bot commented Feb 3, 2022 •

edited

fabricteam commented Feb 3, 2022 •

edited

size-auditor bot commented Feb 3, 2022 •

edited

fabricteam commented Feb 3, 2022 •

edited

Perf Analysis (`@fluentui/react`)

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

Hotell commented Feb 7, 2022 •

edited

micahgodbolt commented Feb 7, 2022 •

edited

micahgodbolt commented Feb 8, 2022 •

edited

micahgodbolt commented Feb 8, 2022 •

edited