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

chore(react-storybook): migrate to new DX stage 1 #19037

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Jul 21, 2021

Pull request checklist

Description of changes

  • updated codeowners

Focus areas to test

(optional)

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 21, 2021

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-accordion
Accordion (including children components)
78.403 kB
23.179 kB
react-avatar
Avatar
54.293 kB
14.665 kB
react-badge
Badge
24.393 kB
7.174 kB
react-badge
CounterBadge
27.206 kB
7.862 kB
react-badge
PresenseBadge
237 B
177 B
react-button
Button
25.967 kB
8.231 kB
react-button
CompoundButton
31.409 kB
9.107 kB
react-button
MenuButton
27.552 kB
8.732 kB
react-button
ToggleButton
36.393 kB
8.907 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
197.875 kB
57.818 kB
react-components
react-components: FluentProvider & webLightTheme
35.513 kB
11.437 kB
react-divider
Divider
15.889 kB
5.747 kB
react-image
Image
10.642 kB
4.264 kB
react-label
Label
28.622 kB
10.654 kB
react-link
Link
14.715 kB
6.012 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 (including children components)
113.933 kB
34.39 kB
react-menu
Menu (including selectable components)
115.945 kB
34.65 kB
react-popover
Popover
140.924 kB
41.969 kB
react-portal
Portal
7.78 kB
2.672 kB
react-positioning
usePopper
23.141 kB
7.931 kB
react-provider
FluentProvider
16.235 kB
5.972 kB
react-theme
Teams: all themes
31.935 kB
6.49 kB
react-theme
Teams: Light theme
19.527 kB
5.504 kB
react-tooltip
Tooltip
44.524 kB
15.221 kB
react-utilities
SSRProvider
213 B
170 B
🤖 This report was generated against e2360defc46ab144907bcd01b12e76fe35c4edbf

@codesandbox-ci
Copy link

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 6ac1c7d:

Sandbox Source
Fluent UI React Starter Configuration

@size-auditor
Copy link

size-auditor bot commented Jul 21, 2021

Asset size changes

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

Baseline commit: e2360defc46ab144907bcd01b12e76fe35c4edbf (build)

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 905 887 5000
BaseButton mount 987 1021 5000
Breadcrumb mount 2783 2721 1000
ButtonNext mount 608 566 5000
Checkbox mount 1714 1729 5000
CheckboxBase mount 1432 1432 5000
ChoiceGroup mount 5223 5391 5000
ComboBox mount 1061 1086 1000
CommandBar mount 11066 11333 1000
ContextualMenu mount 6466 6537 1000
DefaultButton mount 1284 1288 5000
DetailsRow mount 4109 3963 5000
DetailsRowFast mount 4061 4166 5000
DetailsRowNoStyles mount 3890 3924 5000
Dialog mount 2317 2300 1000
DocumentCardTitle mount 177 153 1000
Dropdown mount 3643 3593 5000
FluentProviderNext mount 7714 7849 5000
FocusTrapZone mount 1845 2006 5000
FocusZone mount 1939 1917 5000
IconButton mount 1939 1958 5000
Label mount 353 349 5000
Layer mount 1993 1923 5000
Link mount 538 493 5000
MakeStyles mount 1971 1897 50000
MenuButton mount 1663 1649 5000
MessageBar mount 2244 2281 5000
Nav mount 3581 3713 1000
OverflowSet mount 1094 1077 5000
Panel mount 2198 2223 1000
Persona mount 932 921 1000
Pivot mount 1577 1568 1000
PrimaryButton mount 1477 1405 5000
Rating mount 8738 8749 5000
SearchBox mount 1519 1502 5000
Shimmer mount 2827 2916 5000
Slider mount 2151 2176 5000
SpinButton mount 5298 5422 5000
Spinner mount 473 409 5000
SplitButton mount 3487 3470 5000
Stack mount 563 567 5000
StackWithIntrinsicChildren mount 1782 1811 5000
StackWithTextChildren mount 5251 5217 5000
SwatchColorPicker mount 11448 11155 5000
Tabs mount 1635 1573 1000
TagPicker mount 2774 2675 5000
TeachingBubble mount 12831 12502 5000
Text mount 477 472 5000
TextField mount 1516 1550 5000
ThemeProvider mount 1278 1276 5000
ThemeProvider virtual-rerender 641 649 5000
Toggle mount 858 867 5000
buttonNative mount 121 125 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ListNestedPerf.default 692 615 1.13:1
TreeWith60ListItems.default 203 182 1.12:1
TextMinimalPerf.default 407 375 1.09:1
AnimationMinimalPerf.default 475 445 1.07:1
AttachmentMinimalPerf.default 183 171 1.07:1
HeaderMinimalPerf.default 435 408 1.07:1
ChatDuplicateMessagesPerf.default 347 328 1.06:1
HeaderSlotsPerf.default 902 849 1.06:1
AttachmentSlotsPerf.default 1229 1168 1.05:1
CarouselMinimalPerf.default 545 520 1.05:1
DialogMinimalPerf.default 869 829 1.05:1
DropdownManyItemsPerf.default 811 775 1.05:1
ListWith60ListItems.default 743 706 1.05:1
ButtonMinimalPerf.default 201 194 1.04:1
ButtonSlotsPerf.default 631 605 1.04:1
FlexMinimalPerf.default 328 315 1.04:1
ItemLayoutMinimalPerf.default 1399 1344 1.04:1
RadioGroupMinimalPerf.default 528 510 1.04:1
ReactionMinimalPerf.default 455 439 1.04:1
IconMinimalPerf.default 705 681 1.04:1
TableMinimalPerf.default 464 445 1.04:1
TooltipMinimalPerf.default 1159 1119 1.04:1
TreeMinimalPerf.default 893 857 1.04:1
GridMinimalPerf.default 389 378 1.03:1
MenuButtonMinimalPerf.default 1864 1808 1.03:1
StatusMinimalPerf.default 774 751 1.03:1
ChatWithPopoverPerf.default 424 415 1.02:1
ListCommonPerf.default 729 712 1.02:1
MenuMinimalPerf.default 942 925 1.02:1
PopupMinimalPerf.default 656 641 1.02:1
PortalMinimalPerf.default 195 192 1.02:1
SkeletonMinimalPerf.default 389 380 1.02:1
TableManyItemsPerf.default 2213 2164 1.02:1
CustomToolbarPrototype.default 4288 4196 1.02:1
BoxMinimalPerf.default 397 392 1.01:1
CardMinimalPerf.default 627 621 1.01:1
CheckboxMinimalPerf.default 3025 2982 1.01:1
DropdownMinimalPerf.default 3482 3455 1.01:1
SplitButtonMinimalPerf.default 4326 4301 1.01:1
TextAreaMinimalPerf.default 591 586 1.01:1
VideoMinimalPerf.default 721 712 1.01:1
ButtonOverridesMissPerf.default 1881 1877 1:1
EmbedMinimalPerf.default 4554 4532 1:1
ListMinimalPerf.default 573 573 1:1
LoaderMinimalPerf.default 759 758 1:1
SegmentMinimalPerf.default 379 380 1:1
ToolbarMinimalPerf.default 1035 1030 1:1
AccordionMinimalPerf.default 172 174 0.99:1
ChatMinimalPerf.default 726 730 0.99:1
DatepickerMinimalPerf.default 6614 6667 0.99:1
LabelMinimalPerf.default 437 441 0.99:1
InputMinimalPerf.default 1386 1422 0.97:1
ProviderMergeThemesPerf.default 1771 1830 0.97:1
ProviderMinimalPerf.default 1131 1162 0.97:1
RefMinimalPerf.default 236 243 0.97:1
DividerMinimalPerf.default 411 428 0.96:1
AlertMinimalPerf.default 310 325 0.95:1
AvatarMinimalPerf.default 229 242 0.95:1
LayoutMinimalPerf.default 395 414 0.95:1
RosterPerf.default 1281 1348 0.95:1
FormMinimalPerf.default 457 485 0.94:1
ImageMinimalPerf.default 421 449 0.94:1
SliderMinimalPerf.default 1637 1736 0.94:1

@Hotell Hotell enabled auto-merge (squash) July 21, 2021 11:22
@@ -0,0 +1,11 @@
const rootMain = require('../../../.storybook/main');
Copy link
Member

Choose a reason for hiding this comment

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

Will this package ever have stories?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe in future (to have stories that can be e2e tested)? . it will need a bit overhaul -> migrating from knobs to controls etc

not a big deal - just generator at work ( we can remove it as needed via automation later )

Copy link
Member

Choose a reason for hiding this comment

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

This package really doesn't need either storybook config or API Extractor. Adding these files to new packages where they aren't needed is just increasing the maintenance burden later (and in the case of API Extractor configs, unnecessarily increasing build time).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

really doesn't need either storybook config

please see previous comment

and in the case of API Extractor configs, unnecessarily increasing build time

I'd say this statement is a bit exaggerated (can you provide specific data metrics ) ?

  • all new DX packages should be unified as much as possible for a sake of easier automation. API extractor is a must for typescript declaration rolluping. It is to be determined if it's gonna be used also for documentation generation purposes or not.

is just increasing the maintenance burden

quite contrary. with nx and generators everything is automated and kept in sync ✌️

Copy link
Member

@ecraig12345 ecraig12345 Jul 22, 2021

Choose a reason for hiding this comment

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

really doesn't need either storybook config

please see previous comment

IMO it's a better approach to not have extra junk by default, then only add it if/when needed.

and in the case of API Extractor configs, unnecessarily increasing build time

I'd say this statement is a bit exaggerated (can you provide specific data metrics ) ?

If an API Extractor config is present, the API Extractor generation step runs. Go look for how long that takes in the build. It's probably not huge but cumulatively it is adding a bit of time.

  • all new DX packages should be unified as much as possible for a sake of easier automation. API extractor is a must for typescript declaration rolluping. It is to be determined if it's gonna be used also for documentation generation purposes or not.

Do we actually need declaration rollup for non-component packages?

is just increasing the maintenance burden

quite contrary. with nx and generators everything is automated and kept in sync ✌️

Generators can't magically know which projects should and should not have an API Extractor config. And in general, the existence of generators shouldn't excuse doing things that are otherwise silly/unnecessary.

Copy link
Member

Choose a reason for hiding this comment

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

Search build logs for finished 'api-extractor' in -- it takes at least a few seconds (more for some packages) because it has to build a TS program for each package, plus however long the analysis takes. The only straightforward way to marginally improve that is not using API Extractor where it's not needed. The more comprehensive but less straightforward way (which we may need to investigate at some point if we keep using the tool and want to invest in improving build perf) is creating a shared TS program object and programmatically invoking any tool that needs to use it, instead of using their CLIs.

@Hotell Hotell merged commit f12ec4f into microsoft:master Jul 21, 2021
PeterDraex pushed a commit to PeterDraex/fluentui that referenced this pull request Aug 6, 2021
* chore(react-storybook): migrate to new DX stage 1
* chore: update codeowners for storybook packages
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.

5 participants