fluentui
fluentui copied to clipboard
microsoft
RFC: Stop pre-processing styles with Griffel in `@fluentui/react-components`
Description
The RFC proposes halting the pre-processing of styles in @fluentui/react-components to address CSS clashes observed when apps are bundled into multiple separate bundles.
Two options are presented:
- stopping pre-processing entirely, requiring consuming apps to handle it (breaking change)
- shipping ESM output with unprocessed styles alongside pre-processed styles, maintaining backward compatibility, but increasing complexity and package size
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.
![codesandbox-ci[bot] avatar](https://avatars.githubusercontent.com/in/38409?v=4 "Published by a pub.dev verified publisher"){kind=small}
Perf Analysis (@fluentui/react-components)
No significant results to display.
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 620 | 602 | 5000 | |
| Button | mount | 307 | 297 | 5000 | |
| Field | mount | 1141 | 1086 | 5000 | |
| FluentProvider | mount | 713 | 706 | 5000 | |
| FluentProviderWithTheme | mount | 93 | 81 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 32 | 35 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 72 | 78 | 10 | |
| MakeStyles | mount | 869 | 884 | 50000 | |
| Persona | mount | 1762 | 1686 | 5000 | |
| SpinButton | mount | 1382 | 1408 | 5000 | |
| SwatchPicker | mount | 1673 | 1680 | 5000 |
Perf Analysis (@fluentui/react-northstar)
:warning: 1 potential perf regressions detected
Potential regressions comparing to master
| Scenario | Current PR Ticks | Baseline Ticks | Ratio | Regression Analysis |
|---|---|---|---|---|
| FlexMinimalPerf.default | 163 | 153 | 1.07:1 | analysis |
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| AccordionMinimalPerf.default | 92 | 77 | 1.19:1 |
| AvatarMinimalPerf.default | 115 | 103 | 1.12:1 |
| ChatWithPopoverPerf.default | 207 | 186 | 1.11:1 |
| ButtonMinimalPerf.default | 90 | 82 | 1.1:1 |
| TableMinimalPerf.default | 239 | 221 | 1.08:1 |
| ButtonSlotsPerf.default | 326 | 305 | 1.07:1 |
| HeaderMinimalPerf.default | 212 | 199 | 1.07:1 |
| LabelMinimalPerf.default | 226 | 212 | 1.07:1 |
| DividerMinimalPerf.default | 209 | 197 | 1.06:1 |
| DropdownManyItemsPerf.default | 404 | 382 | 1.06:1 |
| RefMinimalPerf.default | 115 | 108 | 1.06:1 |
| InputMinimalPerf.default | 552 | 524 | 1.05:1 |
| ListNestedPerf.default | 321 | 307 | 1.05:1 |
| ListWith60ListItems.default | 378 | 360 | 1.05:1 |
| PortalMinimalPerf.default | 86 | 82 | 1.05:1 |
| CarouselMinimalPerf.default | 264 | 253 | 1.04:1 |
| RosterPerf.default | 1602 | 1534 | 1.04:1 |
| SkeletonMinimalPerf.default | 199 | 191 | 1.04:1 |
| SplitButtonMinimalPerf.default | 2268 | 2174 | 1.04:1 |
| StatusMinimalPerf.default | 411 | 396 | 1.04:1 |
| GridMinimalPerf.default | 188 | 182 | 1.03:1 |
| LayoutMinimalPerf.default | 203 | 197 | 1.03:1 |
| LoaderMinimalPerf.default | 189 | 184 | 1.03:1 |
| PopupMinimalPerf.default | 347 | 338 | 1.03:1 |
| CheckboxMinimalPerf.default | 1144 | 1120 | 1.02:1 |
| DialogMinimalPerf.default | 443 | 435 | 1.02:1 |
| EmbedMinimalPerf.default | 1868 | 1827 | 1.02:1 |
| ImageMinimalPerf.default | 228 | 224 | 1.02:1 |
| ItemLayoutMinimalPerf.default | 718 | 706 | 1.02:1 |
| MenuMinimalPerf.default | 498 | 486 | 1.02:1 |
| TextMinimalPerf.default | 191 | 188 | 1.02:1 |
| TextAreaMinimalPerf.default | 287 | 280 | 1.02:1 |
| DatepickerMinimalPerf.default | 3526 | 3501 | 1.01:1 |
| ProviderMergeThemesPerf.default | 661 | 653 | 1.01:1 |
| ProviderMinimalPerf.default | 205 | 203 | 1.01:1 |
| ReactionMinimalPerf.default | 211 | 209 | 1.01:1 |
| ToolbarMinimalPerf.default | 552 | 548 | 1.01:1 |
| TreeWith60ListItems.default | 85 | 84 | 1.01:1 |
| AlertMinimalPerf.default | 155 | 155 | 1:1 |
| ListCommonPerf.default | 385 | 384 | 1:1 |
| TreeMinimalPerf.default | 486 | 488 | 1:1 |
| AnimationMinimalPerf.default | 291 | 294 | 0.99:1 |
| AttachmentMinimalPerf.default | 78 | 79 | 0.99:1 |
| BoxMinimalPerf.default | 187 | 188 | 0.99:1 |
| CardMinimalPerf.default | 301 | 303 | 0.99:1 |
| DropdownMinimalPerf.default | 1397 | 1410 | 0.99:1 |
| ListMinimalPerf.default | 299 | 301 | 0.99:1 |
| MenuButtonMinimalPerf.default | 933 | 944 | 0.99:1 |
| SegmentMinimalPerf.default | 191 | 192 | 0.99:1 |
| IconMinimalPerf.default | 388 | 393 | 0.99:1 |
| TableManyItemsPerf.default | 1107 | 1114 | 0.99:1 |
| TooltipMinimalPerf.default | 1271 | 1280 | 0.99:1 |
| VideoMinimalPerf.default | 416 | 421 | 0.99:1 |
| ButtonOverridesMissPerf.default | 633 | 646 | 0.98:1 |
| RadioGroupMinimalPerf.default | 261 | 265 | 0.98:1 |
| CustomToolbarPrototype.default | 1429 | 1456 | 0.98:1 |
| ChatMinimalPerf.default | 434 | 449 | 0.97:1 |
| HeaderSlotsPerf.default | 457 | 469 | 0.97:1 |
| SliderMinimalPerf.default | 716 | 738 | 0.97:1 |
| ChatDuplicateMessagesPerf.default | 153 | 159 | 0.96:1 |
| AttachmentSlotsPerf.default | 615 | 644 | 0.95:1 |
| FormMinimalPerf.default | 210 | 225 | 0.93:1 |
Perf Analysis (@fluentui/react)
No significant results to display.
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 619 | 643 | 5000 | |
| Breadcrumb | mount | 1695 | 1738 | 1000 | |
| Checkbox | mount | 1715 | 1718 | 5000 | |
| CheckboxBase | mount | 1469 | 1488 | 5000 | |
| ChoiceGroup | mount | 2985 | 2957 | 5000 | |
| ComboBox | mount | 689 | 679 | 1000 | |
| CommandBar | mount | 6660 | 6594 | 1000 | |
| ContextualMenu | mount | 14241 | 14262 | 1000 | |
| DefaultButton | mount | 789 | 810 | 5000 | |
| DetailsRow | mount | 2225 | 2230 | 5000 | |
| DetailsRowFast | mount | 2273 | 2255 | 5000 | |
| DetailsRowNoStyles | mount | 2070 | 2045 | 5000 | |
| Dialog | mount | 2790 | 2700 | 1000 | |
| DocumentCardTitle | mount | 230 | 251 | 1000 | |
| Dropdown | mount | 2006 | 1973 | 5000 | |
| FocusTrapZone | mount | 1172 | 1161 | 5000 | |
| FocusZone | mount | 1073 | 1092 | 5000 | |
| GroupedList | mount | 42463 | 42574 | 2 | |
| GroupedList | virtual-rerender | 18255 | 20514 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 51376 | 51904 | 2 | |
| GroupedListV2 | mount | 220 | 223 | 2 | |
| GroupedListV2 | virtual-rerender | 216 | 217 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 225 | 225 | 2 | |
| IconButton | mount | 1200 | 1175 | 5000 | |
| Label | mount | 362 | 347 | 5000 | |
| Layer | mount | 2779 | 2772 | 5000 | |
| Link | mount | 410 | 406 | 5000 | |
| MenuButton | mount | 982 | 1038 | 5000 | |
| MessageBar | mount | 21584 | 21481 | 5000 | |
| Nav | mount | 2060 | 2073 | 1000 | |
| OverflowSet | mount | 782 | 801 | 5000 | |
| Panel | mount | 1777 | 1856 | 1000 | |
| Persona | mount | 758 | 765 | 1000 | |
| Pivot | mount | 905 | 885 | 1000 | |
| PrimaryButton | mount | 926 | 944 | 5000 | |
| Rating | mount | 4748 | 4753 | 5000 | |
| SearchBox | mount | 948 | 945 | 5000 | |
| Shimmer | mount | 1953 | 1926 | 5000 | |
| Slider | mount | 1338 | 1348 | 5000 | |
| SpinButton | mount | 2990 | 3083 | 5000 | |
| Spinner | mount | 393 | 383 | 5000 | |
| SplitButton | mount | 1893 | 1883 | 5000 | |
| Stack | mount | 431 | 424 | 5000 | |
| StackWithIntrinsicChildren | mount | 871 | 904 | 5000 | |
| StackWithTextChildren | mount | 2798 | 2763 | 5000 | |
| SwatchColorPicker | mount | 6455 | 6439 | 5000 | |
| TagPicker | mount | 1499 | 1485 | 5000 | |
| Text | mount | 389 | 390 | 5000 | |
| TextField | mount | 966 | 958 | 5000 | |
| ThemeProvider | mount | 852 | 854 | 5000 | |
| ThemeProvider | virtual-rerender | 592 | 590 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1285 | 1297 | 5000 | |
| Toggle | mount | 623 | 629 | 5000 | |
| buttonNative | mount | 186 | 194 | 5000 |
@levithomason @spmonahan @miroslavstastny @ling1726 @Hotell
The RFC have been updated based on the discussed happened in offline at 24th April.
- Option A was rejected as it will cause breaking changes and will cause performance regressions.
- Instead of prefixing (
.fabdfx=>.prefixfabdfx) we agreed to use prefix as a salt for hash (.fabdfx=>.fvstsq).
The RFC have been updated based on the discussed happened in offline at 24th April.
- Option A was rejected as it will cause breaking changes and will cause performance regressions.
To make things clear from semver POV, if we stop preprocessing our styles, that's not a Breaking Change - no public API surface will change, only thing that will change is that applications might get some performance hit (how big depends case by case, and it this should be clearly communicated to core partners - with intent to onboard them on application level AOT).
regarding the artificial export map additions: I don't think this is good way forward - reasoning provided inline.
things that need to be measured:
- how big is perf hit with current workaround ( prefix of every style )
- how big perf hit are we talking within core partners apps when disabling pre-processing within fluent (react-components)
After we gather proper data and perf hits are significant, we should help onboard core partners to adopt AOT approach within their build systems, as that's the main performance benefit of using our custom css-in-js solution, if folks don't use it they probably don't care about this metric.
After interested parties are onboarded, we should go with solution 1 = stop preprocessing react-components (and also probably archive/delete that babel plugin and docs about libraries pre-processing).
The RFC have been updated based on the discussed happened in offline at 24th April.
- Option A was rejected as it will cause breaking changes and will cause performance regressions.
To make things clear from semver POV, if we stop preprocessing our styles, that's not a Breaking Change - no public API surface will change, only thing that will change is that applications might get some performance hit (how big depends case by case, and it this should be clearly communicated to core partners - with intent to onboard them on application level AOT).
regarding the artificial export map additions: I don't think this is good way forward - reasoning provided inline.
things that need to be measured:
- how big is perf hit with current workaround ( prefix of every style )
- how big perf hit are we talking within core partners apps when disabling pre-processing within fluent (react-components)
After we gather proper data and perf hits are significant, we should help onboard core partners to adopt AOT approach within their build systems, as that's the main performance benefit of using our custom css-in-js solution, if folks don't use it they probably don't care about this metric.
After interested parties are onboarded, we should go with solution 1 = stop preprocessing react-components (and also probably archive/delete that babel plugin and docs about libraries pre-processing).
I had a similar initial intuition, if we could manage to do this in a 2-phase non-breaking change that would be ideal.
- Ship/communicate the right way to process styles, perhaps everyone's already doing this
- Make the change, communicate how to avoid the behavior
We do need to be careful that specificity changes don't become effective breaking changes. This should not break runtime behavior as Griffel would still operate at runtime
We had another offline round of discussions at 22 May.
Outcome:
- We will go with Option A i.e. stop processing
To ensure that it won't break anybody/cause issues:
- We will communicate the change in advance for MS products & onboard them to use AOT
- We will update public docs & communicate it publicly
- Once after that Option A will be executed