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

feat(react-storybook): init new package for convergence storybooks co… #17921

Merged
merged 2 commits into from
Apr 26, 2021

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Apr 22, 2021

…nfiguration

Pull request checklist

Description of changes

  • creates new package @fluentui/react-storybook (this will be used for convergence and beyond)
    • uses new DX/setup via TS path aliases
  • old @fluentui/storybook has it now as dependency so we don't duplicate code, without any breaking changes. Also this package can be used solely with v8/7.

Dep Graph:

image

Focus areas to test

(optional)

@size-auditor
Copy link

size-auditor bot commented Apr 22, 2021

Asset size changes

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

Baseline commit: fdb1af09ba60238bb1c3913af6a6a6fe98f49034 (build)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 22, 2021

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 9f9d7ff:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 22, 2021

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 893 887 5000
BaseButton mount 882 872 5000
Breadcrumb mount 42685 42894 5000
ButtonNext mount 509 515 5000
Checkbox mount 1507 1540 5000
CheckboxBase mount 1258 1256 5000
ChoiceGroup mount 4664 4609 5000
ComboBox mount 977 995 1000
CommandBar mount 9937 9987 1000
ContextualMenu mount 6139 6034 1000
DefaultButton mount 1130 1103 5000
DetailsRow mount 3566 3591 5000
DetailsRowFast mount 3643 3666 5000
DetailsRowNoStyles mount 3437 3394 5000
Dialog mount 1450 1433 1000
DocumentCardTitle mount 1830 1807 1000
Dropdown mount 3139 3148 5000
FocusTrapZone mount 1755 1789 5000
FocusZone mount 1791 1780 5000
IconButton mount 1678 1726 5000
Label mount 338 340 5000
Layer mount 1751 1772 5000
Link mount 450 470 5000
MakeStyles mount 1796 1773 50000
MenuButton mount 1437 1432 5000
MessageBar mount 1975 1992 5000
Nav mount 3239 3227 1000
OverflowSet mount 1034 1052 5000
Panel mount 1398 1404 1000
Persona mount 824 802 1000
Pivot mount 1395 1394 1000
PrimaryButton mount 1233 1242 5000
Rating mount 7514 7447 5000
SearchBox mount 1281 1256 5000
Shimmer mount 2485 2445 5000
Slider mount 1928 1929 5000
SpinButton mount 4906 4888 5000
Spinner mount 415 430 5000
SplitButton mount 3108 3109 5000
Stack mount 489 492 5000
StackWithIntrinsicChildren mount 1514 1517 5000
StackWithTextChildren mount 4424 4434 5000
SwatchColorPicker mount 10075 10167 5000
Tabs mount 1384 1350 1000
TagPicker mount 2370 2358 5000
TeachingBubble mount 11618 11540 5000
Text mount 413 405 5000
TextField mount 1370 1360 5000
ThemeProvider mount 1170 1200 5000
ThemeProvider virtual-rerender 592 597 5000
ThemeProviderNext mount 8991 8975 5000
Toggle mount 789 781 5000
buttonNative mount 115 108 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.17 0.47 0.36:1 2000 332
🦄 Button.Fluent 0.11 0.2 0.55:1 5000 538
🔧 Checkbox.Fluent 0.62 0.35 1.77:1 1000 619
🎯 Dialog.Fluent 0.15 0.21 0.71:1 5000 745
🔧 Dropdown.Fluent 3.07 0.38 8.08:1 1000 3071
🔧 Icon.Fluent 0.13 0.06 2.17:1 5000 644
🦄 Image.Fluent 0.07 0.13 0.54:1 5000 367
🔧 Slider.Fluent 1.56 0.45 3.47:1 1000 1564
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 352
🦄 Tooltip.Fluent 0.14 0.88 0.16:1 5000 710

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 177 163 1.09:1
VideoMinimalPerf.default 641 592 1.08:1
DialogMinimalPerf.default 750 717 1.05:1
TextMinimalPerf.default 361 345 1.05:1
AnimationMinimalPerf.default 431 415 1.04:1
ChatMinimalPerf.default 617 596 1.04:1
FlexMinimalPerf.default 301 290 1.04:1
FormMinimalPerf.default 410 395 1.04:1
ProviderMinimalPerf.default 1016 981 1.04:1
AlertMinimalPerf.default 284 275 1.03:1
AttachmentSlotsPerf.default 1159 1126 1.03:1
BoxMinimalPerf.default 359 347 1.03:1
CardMinimalPerf.default 558 544 1.03:1
CheckboxMinimalPerf.default 2763 2678 1.03:1
LabelMinimalPerf.default 391 380 1.03:1
ButtonMinimalPerf.default 182 179 1.02:1
DividerMinimalPerf.default 362 354 1.02:1
DropdownManyItemsPerf.default 684 673 1.02:1
ImageMinimalPerf.default 378 371 1.02:1
LayoutMinimalPerf.default 377 371 1.02:1
ListNestedPerf.default 562 551 1.02:1
ListWith60ListItems.default 645 630 1.02:1
LoaderMinimalPerf.default 710 699 1.02:1
Dialog.Fluent 745 728 1.02:1
Tooltip.Fluent 710 694 1.02:1
ButtonOverridesMissPerf.default 1670 1660 1.01:1
ButtonUseCssPerf.default 786 781 1.01:1
ButtonUseCssNestingPerf.default 1047 1038 1.01:1
CarouselMinimalPerf.default 464 458 1.01:1
GridMinimalPerf.default 343 341 1.01:1
InputMinimalPerf.default 1257 1244 1.01:1
MenuButtonMinimalPerf.default 1545 1535 1.01:1
RefMinimalPerf.default 255 252 1.01:1
TreeWith60ListItems.default 178 177 1.01:1
Button.Fluent 538 531 1.01:1
Dropdown.Fluent 3071 3036 1.01:1
AvatarMinimalPerf.default 205 205 1:1
DropdownMinimalPerf.default 3070 3065 1:1
ItemLayoutMinimalPerf.default 1245 1247 1:1
RadioGroupMinimalPerf.default 433 435 1:1
ReactionMinimalPerf.default 386 385 1:1
StatusMinimalPerf.default 671 670 1:1
TableManyItemsPerf.default 1887 1879 1:1
TableMinimalPerf.default 407 408 1:1
CustomToolbarPrototype.default 3802 3799 1:1
TooltipMinimalPerf.default 957 956 1:1
Avatar.Fluent 332 332 1:1
Checkbox.Fluent 619 616 1:1
Icon.Fluent 644 647 1:1
DatepickerMinimalPerf.default 45075 45698 0.99:1
EmbedMinimalPerf.default 4062 4086 0.99:1
ListCommonPerf.default 626 631 0.99:1
MenuMinimalPerf.default 867 879 0.99:1
RosterPerf.default 1151 1163 0.99:1
PopupMinimalPerf.default 710 714 0.99:1
PortalMinimalPerf.default 171 173 0.99:1
ProviderMergeThemesPerf.default 1643 1668 0.99:1
SegmentMinimalPerf.default 351 353 0.99:1
SplitButtonMinimalPerf.default 3654 3677 0.99:1
IconMinimalPerf.default 604 612 0.99:1
ChatDuplicateMessagesPerf.default 282 289 0.98:1
HeaderMinimalPerf.default 361 367 0.98:1
HeaderSlotsPerf.default 753 766 0.98:1
SkeletonMinimalPerf.default 355 363 0.98:1
SliderMinimalPerf.default 1571 1604 0.98:1
TreeMinimalPerf.default 764 778 0.98:1
Slider.Fluent 1564 1593 0.98:1
Text.Fluent 352 360 0.98:1
ChatWithPopoverPerf.default 369 382 0.97:1
ListMinimalPerf.default 496 510 0.97:1
TextAreaMinimalPerf.default 466 482 0.97:1
ButtonSlotsPerf.default 533 555 0.96:1
ToolbarMinimalPerf.default 916 951 0.96:1
AccordionMinimalPerf.default 158 167 0.95:1
Image.Fluent 367 387 0.95:1

@Hotell Hotell marked this pull request as ready for review April 22, 2021 18:13
@Hotell Hotell requested review from JustSlone and a team as code owners April 22, 2021 18:13
"just": "just-scripts",
"lint": "just-scripts lint",
"start": "just-scripts dev:storybook",
"start-test": "echo \"This is DEPRECATED instead use 'test --watch'\" && just-scripts jest-watch",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you can just get rid of it for new packages

Suggested change
"start-test": "echo \"This is DEPRECATED instead use 'test --watch'\" && just-scripts jest-watch",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is gonna be handled in batch/automation scripts per package when new Dx is ready

"start": "just-scripts dev:storybook",
"start-test": "echo \"This is DEPRECATED instead use 'test --watch'\" && just-scripts jest-watch",
"test": "jest",
"update-snapshots": "echo \"This is DEPRECATED instead use 'test -u'\" && just-scripts jest -u"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No snapshots here I think... also we should just stop using snapshot tests IMO

Suggested change
"update-snapshots": "echo \"This is DEPRECATED instead use 'test -u'\" && just-scripts jest -u"

Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, a couple small suggestions (and agreed with Shift and Ling's comments)

Comment on lines +103 to +104
packages/react-storybook/ @cxe-prg @ling1726 @layershifter
packages/storybook/ @cxe-prg @ling1726 @layershifter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should both of these (or at least the old one) also have the build team as an owner?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see such a need ( I'd like to have our team and teams fluent team here for now ).
both are private for now, would be nice to shape up the new one to be a proper addon etc - then we can make it public.

@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want it's probably okay to get rid of API Extractor for this package

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im gonna keep it for consistency reasons. in general every package should use common tooling in terms of rolluping types and producing docs.

@Hotell Hotell force-pushed the hotell/16889/new-react-storybook-pkg branch 2 times, most recently from 0f87eb4 to 90bc73c Compare April 23, 2021 16:26
@Hotell Hotell enabled auto-merge (squash) April 23, 2021 16:27
@Hotell Hotell force-pushed the hotell/16889/new-react-storybook-pkg branch from b58420b to 9f9d7ff Compare April 26, 2021 15:30
@Hotell Hotell merged commit ebadd46 into microsoft:master Apr 26, 2021
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
microsoft#17921)

* feat(react-storybook): init new package for convergence storybooks configuration
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.

8 participants