react-avatar: migration to new DX by tringakrasniqi · Pull Request #18866 · microsoft/fluentui

tringakrasniqi · 2021-07-08T14:25:46Z

Pull request checklist

Addresses an existing issue: Fixes partially migration: migrate converged packages to new DX (1st stage) #18579
Include a change request file using $ yarn change

Description of changes

Migration to new DX
Removed dependencies on v8 for the react-avatar storybook

codesandbox-ci · 2021-07-08T14:56:20Z

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 cda51fc:

Sandbox	Source
Fluent UI React Starter	Configuration

Co-authored-by: André <andrefcdias@users.noreply.github.com>

Hotell · 2021-07-12T11:54:56Z

this PR is failing because of babel-make-styles plugin:

Current:

After removing make styles plugin from babelrc:

↓↓↓↓

yarn lage build --to @fluentui/react-avatar

↓↓↓↓

can you help us out @layershifter ?

there is also another issue, that is caused by make-styles - it uses ES2015+ language features, thus API extractor will properly notify us that something is not ok. (this was reported/discovered by @ling1726 )

You'll need to change lib to es2015

layershifter · 2021-07-12T12:37:01Z

this PR is failing because of babel-make-styles plugin:

Current:

After removing make styles plugin from babelrc:

↓↓↓↓

yarn lage build --to @fluentui/react-avatar

↓↓↓↓

can you help us out @layershifter ?

This looks like an issue with @linaria/shaker, I will debug the problem and report it. Currently, to unblock the PR I suggest to apply the following change:

  const rootClasses = styles.root;
  const svgClasses = styles.svg;

  return { containerProps, nativeProps, rootClasses, svgClasses };
};

+export const renderIcon = (
-const renderIcon = (
  SVGElement: (props: { svgClasses: string }) => JSX.Element,
): React.FC<React.HTMLAttributes<HTMLSpanElement>> => props => {
  const { containerProps, nativeProps, rootClasses, svgClasses } = useIconProps(props);

Hotell · 2021-07-12T13:38:14Z

created issue for better tracking #18903 @layershifter

size-auditor · 2021-07-12T14:06:12Z

Asset size changes

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

Baseline commit: 33094c23d24a0a1610b6d182d2922dd51101b508 (build)

fabricteam · 2021-07-12T14:19:07Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	804	787	5000
BaseButton	mount	905	895	5000
Breadcrumb	mount	2598	2634	1000
ButtonNext	mount	527	540	5000
Checkbox	mount	1496	1499	5000
CheckboxBase	mount	1266	1289	5000
ChoiceGroup	mount	4744	4679	5000
ComboBox	mount	1016	968	1000
CommandBar	mount	10054	10072	1000
ContextualMenu	mount	6179	6250	1000
DefaultButton	mount	1095	1119	5000
DetailsRow	mount	3613	3650	5000
DetailsRowFast	mount	3699	3664	5000
DetailsRowNoStyles	mount	3463	3522	5000
Dialog	mount	2130	2134	1000
DocumentCardTitle	mount	136	148	1000
Dropdown	mount	3167	3176	5000
FluentProviderNext	mount	7197	7198	5000
FocusTrapZone	mount	1755	1762	5000
FocusZone	mount	1807	1784	5000
IconButton	mount	1705	1693	5000
Label	mount	330	330	5000
Layer	mount	1776	1783	5000
Link	mount	455	463	5000
MakeStyles	mount	1821	1813	50000
MenuButton	mount	1454	1466	5000
MessageBar	mount	1979	2014	5000
Nav	mount	3207	3174	1000
OverflowSet	mount	1018	1016	5000
Panel	mount	2038	2057	1000
Persona	mount	799	806	1000
Pivot	mount	1384	1375	1000
PrimaryButton	mount	1273	1270	5000
Rating	mount	7500	7547	5000
SearchBox	mount	1273	1295	5000
Shimmer	mount	2471	2557	5000
Slider	mount	1965	1956	5000
SpinButton	mount	4888	5017	5000
Spinner	mount	421	423	5000
SplitButton	mount	3108	3084	5000
Stack	mount	499	496	5000
StackWithIntrinsicChildren	mount	1494	1473	5000
StackWithTextChildren	mount	4431	4393	5000
SwatchColorPicker	mount	9967	10140	5000
Tabs	mount	1396	1481	1000
TagPicker	mount	2366	2364	5000
TeachingBubble	mount	11749	11770	5000
Text	mount	424	418	5000
TextField	mount	1347	1373	5000
ThemeProvider	mount	1169	1187	5000
ThemeProvider	virtual-rerender	591	585	5000
Toggle	mount	820	794	5000
buttonNative	mount	108	115	5000

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

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AccordionMinimalPerf.default	147	138	1.07:1
ListCommonPerf.default	634	600	1.06:1
CardMinimalPerf.default	542	516	1.05:1
HeaderMinimalPerf.default	358	342	1.05:1
AttachmentMinimalPerf.default	153	147	1.04:1
ButtonSlotsPerf.default	526	508	1.04:1
LabelMinimalPerf.default	384	370	1.04:1
ListMinimalPerf.default	509	490	1.04:1
SkeletonMinimalPerf.default	344	331	1.04:1
DialogMinimalPerf.default	755	736	1.03:1
LayoutMinimalPerf.default	357	348	1.03:1
RadioGroupMinimalPerf.default	443	431	1.03:1
TableMinimalPerf.default	401	388	1.03:1
ButtonOverridesMissPerf.default	1663	1637	1.02:1
DividerMinimalPerf.default	347	339	1.02:1
InputMinimalPerf.default	1251	1221	1.02:1
PortalMinimalPerf.default	176	172	1.02:1
TreeMinimalPerf.default	787	772	1.02:1
AnimationMinimalPerf.default	406	402	1.01:1
CarouselMinimalPerf.default	457	453	1.01:1
FormMinimalPerf.default	386	382	1.01:1
GridMinimalPerf.default	332	329	1.01:1
ListNestedPerf.default	552	544	1.01:1
MenuButtonMinimalPerf.default	1624	1609	1.01:1
PopupMinimalPerf.default	589	582	1.01:1
RefMinimalPerf.default	225	222	1.01:1
IconMinimalPerf.default	602	597	1.01:1
TableManyItemsPerf.default	1863	1836	1.01:1
TextMinimalPerf.default	343	338	1.01:1
CustomToolbarPrototype.default	3768	3744	1.01:1
ToolbarMinimalPerf.default	919	913	1.01:1
TooltipMinimalPerf.default	999	987	1.01:1
AlertMinimalPerf.default	265	265	1:1
BoxMinimalPerf.default	336	337	1:1
ChatMinimalPerf.default	635	635	1:1
CheckboxMinimalPerf.default	2690	2686	1:1
DropdownMinimalPerf.default	3059	3058	1:1
EmbedMinimalPerf.default	4037	4022	1:1
HeaderSlotsPerf.default	739	742	1:1
ImageMinimalPerf.default	358	357	1:1
LoaderMinimalPerf.default	671	670	1:1
MenuMinimalPerf.default	827	830	1:1
SplitButtonMinimalPerf.default	3708	3716	1:1
StatusMinimalPerf.default	663	662	1:1
TextAreaMinimalPerf.default	485	486	1:1
AvatarMinimalPerf.default	188	189	0.99:1
ButtonMinimalPerf.default	160	162	0.99:1
ChatWithPopoverPerf.default	339	342	0.99:1
ProviderMinimalPerf.default	946	960	0.99:1
ReactionMinimalPerf.default	366	370	0.99:1
SliderMinimalPerf.default	1543	1552	0.99:1
AttachmentSlotsPerf.default	1028	1047	0.98:1
DatepickerMinimalPerf.default	5203	5313	0.98:1
FlexMinimalPerf.default	273	280	0.98:1
ItemLayoutMinimalPerf.default	1181	1202	0.98:1
ListWith60ListItems.default	629	639	0.98:1
ProviderMergeThemesPerf.default	1636	1676	0.98:1
SegmentMinimalPerf.default	328	336	0.98:1
DropdownManyItemsPerf.default	655	684	0.96:1
RosterPerf.default	1094	1144	0.96:1
ChatDuplicateMessagesPerf.default	265	285	0.93:1
VideoMinimalPerf.default	572	616	0.93:1
TreeWith60ListItems.default	160	174	0.92:1

Hotell

fantastic job! thank you

tringakrasniqi · 2021-07-12T14:30:08Z

packages/react-avatar/src/Avatar.stories.tsx

    useValueSelector('size', useValueSelectorState(examples.size, 96), true),
    useValueSelector('square', useValueSelectorState([true, false])),
-    useValueSelector('badge', useValueSelectorState(examples.badge), false, badgeToString),
+    useValueSelector('badge', useValueSelectorState(examples.badge), false, badgeToString as () => string),


Added as () => string as a temp workaround to fix TS type error as this story will need to be replaced at some point to the storybook controls.

behowell

Looks good, just some minor comments. Thanks for updating the react-avatar package!

packages/react-avatar/src/Avatar.stories.tsx

behowell · 2021-07-14T10:24:13Z

packages/react-avatar/src/Avatar.stories.tsx

+          <Button onClick={React.useCallback(() => setActive(a => !a), [])}>Toggle Active</Button>
+          {/* Temporarly replacement of SpinButtons */}
+          <div className={flexStyles.flex}>
+            Active Display:


It'd be helpful for this to display the current value of the prop:

Suggested change

Active Display:

activeDisplay="{activeDisplay}"

True, I changed them to this -> <span>activeDisplay="{activeDisplay}"</span>

packages/react-avatar/src/Avatar.stories.tsx

fabricteam · 2021-07-14T15:47:21Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-divider` Divider	`15.889 kB` `5.747 kB`
`react-image` Image	`10.642 kB` `4.264 kB`
`react-make-styles` makeStaticStyles (runtime)	`7.59 kB` `3.321 kB`
`react-make-styles` makeStyles + mergeClasses (runtime)	`22.135 kB` `8.356 kB`
`react-make-styles` makeStyles + mergeClasses (build time)	`2.557 kB` `1.202 kB`
`react-menu` Menu - Default	`113.947 kB` `34.389 kB`
`react-menu` Menu - Selectable	`115.959 kB` `34.649 kB`
`react-popover` Popover	`140.938 kB` `41.968 kB`
`react-theme` Teams: all themes	`31.935 kB` `6.49 kB`
`react-theme` Teams: Light theme	`19.527 kB` `5.504 kB`

_{🤖 This report was generated against 33094c23d24a0a1610b6d182d2922dd51101b508}

Hotell · 2021-07-14T15:54:28Z

weird, the pipeline is failing on false assumption - you need to provide change file for @fluentui/react which you didn't change at all in this PR.

~~any thoughts why is this happening @ecraig12345 ?~~

nevermind - react-examples is now private, so you need to manually remove this change file

Hotell · 2021-07-14T17:03:17Z

issues is still present:

Found changes in the following packages: 
  @fluentui/react

NOTE:

there are no changes to react package in this PR
I forgot how beachball handles this situations when package is being set to private and change already exists in other PR's 🔥

any thoughts what should be done @ecraig12345 ? 🙏

layershifter · 2021-07-14T17:24:59Z

issues is still present:
Found changes in the following packages: 
  @fluentui/react
NOTE:

there are no changes to react package in this PR

I forgot how beachball handles this situations when package is being set to private and change already exists in other PR's 🔥

any thoughts what should be done @ecraig12345 ? 🙏

It's not related to beachball, this PR contains formatting change in packages/react/src/components/ContextualMenu/ContextualMenu.base.tsx ¯_(ツ)_/¯

It looks that it happened during merge (385f055) and came from #18853.

Hotell · 2021-07-14T17:29:25Z

issues is still present:
Found changes in the following packages: 
  @fluentui/react
NOTE:

there are no changes to react package in this PR

I forgot how beachball handles this situations when package is being set to private and change already exists in other PR's 🔥

any thoughts what should be done @ecraig12345 ? 🙏
It's not related to beachball, this PR contains formatting change in packages/react/src/components/ContextualMenu/ContextualMenu.base.tsx

well, I overlooked that one. thanks @layershifter . Bad rebase @tringakrasniqi ?

anyways if I tried to run change on my local machine I was still getting errors from beachball

ecraig12345 · 2021-07-14T17:35:52Z

packages/react/src/components/ContextualMenu/ContextualMenu.base.tsx

        ariaLabellledby = id;
      } else {
-        const id = sectionProps.title.id || (this._id + sectionProps.title.key.replace(/\s/g, ''));
+        const id = sectionProps.title.id || this._id + sectionProps.title.key.replace(/\s/g, '');


This was because some incorrect formatting got checked in with another PR. If you merge with master it should be fixed now.

* DX migration: partial fix of Storybook issues * DX migration: partial fix of Storybook * Fixes for avatar storybook: Avatar Playground story * Change files * Fix for the Avatar Playground stories Co-authored-by: André <andrefcdias@users.noreply.github.com> * Added fixes for build errors * Added api report file: etc/react-avatar.api.md * Changed description of props in ActiveAnimation story * Deleted @fluentui-react-examples change file Co-authored-by: André <andrefcdias@users.noreply.github.com>

DX migration: partial fix of Storybook issues

31b4f4c

msft-fluent-ui-bot added the Component: Avatar label Jul 8, 2021

msft-fluent-ui-bot assigned ling1726 Jul 8, 2021

DX migration: partial fix of Storybook

fb623c6

tringakrasniqi added 3 commits July 8, 2021 17:56

Fixes for avatar storybook: Avatar Playground story

62692f8

Merge branch 'master' into dx-migration-avatar

2c971b8

Change files

a4b617f

tringakrasniqi changed the title ~~DX migration: partial fix of Storybook issues~~ react-avatar: migration to new DX Jul 9, 2021

tringakrasniqi and others added 3 commits July 9, 2021 09:57

Merge branch 'master' into dx-migration-avatar

274d658

Merge branch 'master' into dx-migration-avatar

29a6c87

Fix for the Avatar Playground stories

1347032

Co-authored-by: André <andrefcdias@users.noreply.github.com>

tringakrasniqi added the Area: Build System label Jul 12, 2021

Added fixes for build errors

918edc1

Hotell mentioned this pull request Jul 12, 2021

babel-make-styles plugin causing build failures after migrating react-avatar to new DX #18903

Closed

Added api report file: etc/react-avatar.api.md

ffe9503

Hotell approved these changes Jul 12, 2021

View reviewed changes

tringakrasniqi marked this pull request as ready for review July 12, 2021 14:22

tringakrasniqi requested review from a team and behowell as code owners July 12, 2021 14:22

tringakrasniqi mentioned this pull request Jul 12, 2021

migration: migrate converged packages to new DX (1st stage) #18579

Closed

32 tasks

tringakrasniqi commented Jul 12, 2021

View reviewed changes

Hotell self-requested a review July 12, 2021 14:30

Hotell approved these changes Jul 12, 2021

View reviewed changes

Hotell added the Fluent UI react-components (v9) label Jul 12, 2021

Hotell added this to the July Project Cycle Q2 2021 milestone Jul 12, 2021

Merge branch 'master' into dx-migration-avatar

b091e33

behowell approved these changes Jul 14, 2021

View reviewed changes

tringakrasniqi added 2 commits July 14, 2021 14:42

Merge branch 'master' into dx-migration-avatar

385f055

Changed description of props in ActiveAnimation story

cec2e11

Deleted @fluentui-react-examples change file

4334511

ecraig12345 reviewed Jul 14, 2021

View reviewed changes

tringakrasniqi added 2 commits July 15, 2021 10:39

Merge branch 'master' into dx-migration-avatar

0912a9f

Merge branch 'dx-migration-avatar' into dx-migration-avatar

cda51fc

tringakrasniqi merged commit bafb9f1 into microsoft:master Jul 15, 2021

tringakrasniqi deleted the dx-migration-avatar branch July 15, 2021 09:25

Conversation

tringakrasniqi commented Jul 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Uh oh!

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

Uh oh!

Uh oh!

Hotell commented Jul 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

layershifter commented Jul 12, 2021

Uh oh!

Hotell commented Jul 12, 2021

Uh oh!

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

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Jul 12, 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!

Hotell left a comment

Choose a reason for hiding this comment

Uh oh!

tringakrasniqi Jul 12, 2021

Choose a reason for hiding this comment

Uh oh!

behowell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

behowell Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

tringakrasniqi Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

📊 Bundle size report

Uh oh!

Hotell commented Jul 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hotell commented Jul 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

layershifter commented Jul 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hotell commented Jul 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ecraig12345 Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tringakrasniqi commented Jul 8, 2021 •

edited

Loading

codesandbox-ci bot commented Jul 8, 2021 •

edited

Loading

Hotell commented Jul 12, 2021 •

edited

Loading

size-auditor bot commented Jul 12, 2021 •

edited

Loading

fabricteam commented Jul 12, 2021 •

edited

Loading

Perf Analysis (`@fluentui/react`)

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

fabricteam commented Jul 14, 2021 •

edited

Loading

Hotell commented Jul 14, 2021 •

edited

Loading

Hotell commented Jul 14, 2021 •

edited

Loading

layershifter commented Jul 14, 2021 •

edited

Loading

Hotell commented Jul 14, 2021 •

edited

Loading