Add shadow DOM support to @fluentui/react (Fluent v8)
#30689
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spmonahan
requested review from
a team,
khmakoto,
behowell,
ThomasMichon,
phkuo,
dzearing and
micahgodbolt
as code owners
|
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. |
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| TreeWith60ListItems.default | 96 | 85 | 1.13:1 |
| ChatWithPopoverPerf.default | 201 | 182 | 1.1:1 |
| AttachmentMinimalPerf.default | 79 | 74 | 1.07:1 |
| AttachmentSlotsPerf.default | 633 | 597 | 1.06:1 |
| ChatDuplicateMessagesPerf.default | 157 | 148 | 1.06:1 |
| ImageMinimalPerf.default | 232 | 218 | 1.06:1 |
| InputMinimalPerf.default | 564 | 531 | 1.06:1 |
| ListCommonPerf.default | 397 | 373 | 1.06:1 |
| RefMinimalPerf.default | 115 | 108 | 1.06:1 |
| DividerMinimalPerf.default | 212 | 201 | 1.05:1 |
| PortalMinimalPerf.default | 87 | 83 | 1.05:1 |
| ListMinimalPerf.default | 310 | 299 | 1.04:1 |
| MenuMinimalPerf.default | 511 | 493 | 1.04:1 |
| ProviderMinimalPerf.default | 204 | 197 | 1.04:1 |
| TableMinimalPerf.default | 237 | 228 | 1.04:1 |
| VideoMinimalPerf.default | 443 | 425 | 1.04:1 |
| BoxMinimalPerf.default | 195 | 189 | 1.03:1 |
| ButtonMinimalPerf.default | 92 | 89 | 1.03:1 |
| DialogMinimalPerf.default | 450 | 439 | 1.03:1 |
| TreeMinimalPerf.default | 487 | 474 | 1.03:1 |
| CarouselMinimalPerf.default | 261 | 255 | 1.02:1 |
| DropdownMinimalPerf.default | 1444 | 1416 | 1.02:1 |
| HeaderSlotsPerf.default | 471 | 460 | 1.02:1 |
| PopupMinimalPerf.default | 347 | 339 | 1.02:1 |
| SkeletonMinimalPerf.default | 203 | 199 | 1.02:1 |
| StatusMinimalPerf.default | 394 | 385 | 1.02:1 |
| ButtonOverridesMissPerf.default | 649 | 645 | 1.01:1 |
| CheckboxMinimalPerf.default | 1117 | 1104 | 1.01:1 |
| GridMinimalPerf.default | 192 | 190 | 1.01:1 |
| HeaderMinimalPerf.default | 210 | 207 | 1.01:1 |
| ItemLayoutMinimalPerf.default | 720 | 712 | 1.01:1 |
| SplitButtonMinimalPerf.default | 2242 | 2225 | 1.01:1 |
| TextMinimalPerf.default | 195 | 193 | 1.01:1 |
| AvatarMinimalPerf.default | 105 | 105 | 1:1 |
| DropdownManyItemsPerf.default | 375 | 374 | 1:1 |
| FlexMinimalPerf.default | 158 | 158 | 1:1 |
| LabelMinimalPerf.default | 219 | 219 | 1:1 |
| RadioGroupMinimalPerf.default | 251 | 250 | 1:1 |
| TooltipMinimalPerf.default | 1284 | 1282 | 1:1 |
| DatepickerMinimalPerf.default | 3513 | 3541 | 0.99:1 |
| LayoutMinimalPerf.default | 196 | 198 | 0.99:1 |
| LoaderMinimalPerf.default | 182 | 184 | 0.99:1 |
| MenuButtonMinimalPerf.default | 931 | 945 | 0.99:1 |
| ReactionMinimalPerf.default | 205 | 207 | 0.99:1 |
| TableManyItemsPerf.default | 1085 | 1101 | 0.99:1 |
| ToolbarMinimalPerf.default | 532 | 537 | 0.99:1 |
| AccordionMinimalPerf.default | 89 | 91 | 0.98:1 |
| AnimationMinimalPerf.default | 293 | 299 | 0.98:1 |
| ButtonSlotsPerf.default | 323 | 328 | 0.98:1 |
| ChatMinimalPerf.default | 430 | 438 | 0.98:1 |
| EmbedMinimalPerf.default | 1846 | 1887 | 0.98:1 |
| ListNestedPerf.default | 312 | 320 | 0.98:1 |
| RosterPerf.default | 1537 | 1572 | 0.98:1 |
| SliderMinimalPerf.default | 725 | 739 | 0.98:1 |
| TextAreaMinimalPerf.default | 279 | 285 | 0.98:1 |
| CustomToolbarPrototype.default | 1446 | 1474 | 0.98:1 |
| AlertMinimalPerf.default | 162 | 167 | 0.97:1 |
| ListWith60ListItems.default | 347 | 356 | 0.97:1 |
| FormMinimalPerf.default | 213 | 222 | 0.96:1 |
| ProviderMergeThemesPerf.default | 640 | 670 | 0.96:1 |
| SegmentMinimalPerf.default | 195 | 203 | 0.96:1 |
| IconMinimalPerf.default | 359 | 380 | 0.94:1 |
| CardMinimalPerf.default | 288 | 312 | 0.92:1 |
📊 Bundle size report
Unchanged fixtures
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 626 | 642 | 5000 | |
| Breadcrumb | mount | 1729 | 1708 | 1000 | |
| Checkbox | mount | 1684 | 1696 | 5000 | |
| CheckboxBase | mount | 1475 | 1512 | 5000 | |
| ChoiceGroup | mount | 2930 | 3007 | 5000 | |
| ComboBox | mount | 654 | 669 | 1000 | |
| CommandBar | mount | 6221 | 6635 | 1000 | |
| ContextualMenu | mount | 13553 | 13838 | 1000 | |
| DefaultButton | mount | 742 | 820 | 5000 | |
| DetailsRow | mount | 2180 | 2225 | 5000 | |
| DetailsRowFast | mount | 2208 | 2238 | 5000 | |
| DetailsRowNoStyles | mount | 2029 | 2059 | 5000 | |
| Dialog | mount | 2670 | 2690 | 1000 | |
| DocumentCardTitle | mount | 230 | 239 | 1000 | |
| Dropdown | mount | 1978 | 2025 | 5000 | |
| FocusTrapZone | mount | 1150 | 1140 | 5000 | |
| FocusZone | mount | 1081 | 1087 | 5000 | |
| GroupedList | mount | 37468 | 42295 | 2 | |
| GroupedList | virtual-rerender | 20045 | 20359 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 50807 | 51958 | 2 | |
| GroupedListV2 | mount | 246 | 232 | 2 | |
| GroupedListV2 | virtual-rerender | 220 | 210 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 227 | 230 | 2 | |
| IconButton | mount | 1091 | 1148 | 5000 | |
| Label | mount | 347 | 356 | 5000 | |
| Layer | mount | 2704 | 2773 | 5000 | |
| Link | mount | 403 | 399 | 5000 | |
| MenuButton | mount | 928 | 964 | 5000 | |
| MessageBar | mount | 21618 | 21640 | 5000 | |
| Nav | mount | 1951 | 2062 | 1000 | |
| OverflowSet | mount | 780 | 790 | 5000 | |
| Panel | mount | 1855 | 1780 | 1000 | |
| Persona | mount | 715 | 752 | 1000 | |
| Pivot | mount | 872 | 922 | 1000 | |
| PrimaryButton | mount | 852 | 928 | 5000 | |
| Rating | mount | 4642 | 4674 | 5000 | |
| SearchBox | mount | 897 | 951 | 5000 | |
| Shimmer | mount | 1853 | 1906 | 5000 | |
| Slider | mount | 1328 | 1335 | 5000 | |
| SpinButton | mount | 2911 | 3014 | 5000 | |
| Spinner | mount | 393 | 401 | 5000 | |
| SplitButton | mount | 1896 | 1899 | 5000 | |
| Stack | mount | 412 | 435 | 5000 | |
| StackWithIntrinsicChildren | mount | 858 | 862 | 5000 | |
| StackWithTextChildren | mount | 2622 | 2662 | 5000 | |
| SwatchColorPicker | mount | 6248 | 6424 | 5000 | |
| TagPicker | mount | 1449 | 1524 | 5000 | |
| Text | mount | 374 | 382 | 5000 | |
| TextField | mount | 919 | 930 | 5000 | |
| ThemeProvider | mount | 837 | 848 | 5000 | |
| ThemeProvider | virtual-rerender | 570 | 585 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1280 | 1280 | 5000 | |
| Toggle | mount | 634 | 625 | 5000 | |
| buttonNative | mount | 197 | 199 | 5000 |
Hotell
reviewed
Mar 4, 2024
|
The bundle size increase all around the v8 are bit concerning tbh, have we measured the performance impact for users using v8 but not having need to ulitize web-components apis ? wondering the effort of making the implementation be opt-in rather than all in ? |
The existing perf tests do this because they are not using shadow DOM.
The shadow DOM is opt in but, obviously, the code is not🙁 Looking into options to trim this back. |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| FluentProviderWithTheme | virtual-rerender | 35 | 28 | 10 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 621 | 660 | 5000 | |
| Button | mount | 295 | 296 | 5000 | |
| Field | mount | 1151 | 1120 | 5000 | |
| FluentProvider | mount | 693 | 706 | 5000 | |
| FluentProviderWithTheme | mount | 86 | 82 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 35 | 28 | 10 | Possible regression |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 73 | 82 | 10 | |
| MakeStyles | mount | 840 | 855 | 50000 | |
| Persona | mount | 1770 | 1720 | 5000 | |
| SpinButton | mount | 1382 | 1438 | 5000 | |
| SwatchPicker | mount | 1546 | 1521 | 5000 |
🕵 fluentuiv9 No visual regressions between this PR and main |
@Hotell First: thank you for taking the time to review! I know this is a gigantic PR so I really appreciate the input! I've refactored this PR quite a bit to bring the bundle size impact down. There will be some impact across the board because it's not possible to swap out the LMK what you think about this refactor and if you have any ideas to push the bundle size impact lower. That said, I feel the bundle impact is acceptable given this is a feature several teams are interested in and there are no runtime performance regressions. |
khmakoto
reviewed
Mar 26, 2024
change/@fluentui-style-utilities-febc26a4-57df-4d2a-b99f-b4d43752dd6e.json
Outdated
Show resolved
Hide resolved
khmakoto
reviewed
Mar 27, 2024
khmakoto
approved these changes
Mar 27, 2024
khmakoto
reviewed
Mar 27, 2024
Co-authored-by: KHMakoto <Humberto.Morimoto@microsoft.com>
Co-authored-by: KHMakoto <Humberto.Morimoto@microsoft.com>
This was added in #31110. I've run it locally to get an initial result while investigating another way to push the bundle size down:
For the entire library the bundle size diff is |
fabricteam
reviewed
Apr 18, 2024
micahgodbolt
approved these changes
Apr 18, 2024
|
@Hotell I looked into what we might do about pushing the bundle size down in the styles. Ultimately I don't think it's worth the effort. The styles are plain JS functions so we cannot take advantage of React context to know if the style is being rendered inside a shadow root or not so we'd have to update the functions and plumb this state through. In addition to the added complexity of doing this it'll cut into and bundle size savings we'd achieve. From a bundle size point of view I think it'd be a net positive but from a complexity point of view I think it'd be a negative. If we need to revisit in the future it's always something we could implement later. |
marcosmoura
added a commit
to marcosmoura/fluentui
that referenced
this pull request
Apr 23, 2024
* master: (416 commits) fix: remove relative imports within stories which are invalid for creating 'show docs' and circular dep imports from cypress files (microsoft#31087) Fix overlapping bars on continuous axes (microsoft#31035) feat(scripts-tasks): implement ~36% faster type-check task (microsoft#31116) feat(workspace-plugin): implement split-library-in-two migration generator (microsoft#31086) applying package updates Sankey diagram: Support number formatting (microsoft#31113) Fix wrong position of hover callout in case of single data AreaChart (microsoft#30256) Combobox filtering bug fix (microsoft#31141) chore(react-tag-picker): adds text elliptical clipping example (microsoft#31114) docs: command cheat sheet (microsoft#30685) chore: refactor tests for createPresenceComponent() (microsoft#31137) Stable Release: TeachingPopover (microsoft#31112) applying package updates fix: SpinButton buttons now show correct visuals at bounds (microsoft#31126) feat: add accessibility docs to Link about color/underlines (microsoft#31121) fix: Table and DataGrid should not remove cells from the accessibility tree (microsoft#31068) Add shadow DOM support to `@fluentui/react` (Fluent v8) (microsoft#30689) fix(react-swatch-picker): fixes after bug bash (microsoft#31097) feat: unify v9 babel preset in all packages (microsoft#31088) applying package updates ...
marcosmoura
added a commit
to marcosmoura/fluentui
that referenced
this pull request
Apr 23, 2024
* master: (416 commits) fix: remove relative imports within stories which are invalid for creating 'show docs' and circular dep imports from cypress files (microsoft#31087) Fix overlapping bars on continuous axes (microsoft#31035) feat(scripts-tasks): implement ~36% faster type-check task (microsoft#31116) feat(workspace-plugin): implement split-library-in-two migration generator (microsoft#31086) applying package updates Sankey diagram: Support number formatting (microsoft#31113) Fix wrong position of hover callout in case of single data AreaChart (microsoft#30256) Combobox filtering bug fix (microsoft#31141) chore(react-tag-picker): adds text elliptical clipping example (microsoft#31114) docs: command cheat sheet (microsoft#30685) chore: refactor tests for createPresenceComponent() (microsoft#31137) Stable Release: TeachingPopover (microsoft#31112) applying package updates fix: SpinButton buttons now show correct visuals at bounds (microsoft#31126) feat: add accessibility docs to Link about color/underlines (microsoft#31121) fix: Table and DataGrid should not remove cells from the accessibility tree (microsoft#31068) Add shadow DOM support to `@fluentui/react` (Fluent v8) (microsoft#30689) fix(react-swatch-picker): fixes after bug bash (microsoft#31097) feat: unify v9 babel preset in all packages (microsoft#31088) applying package updates ...
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previous Behavior
@fluentui/reactwould not correctly render styles in shadow DOM.New Behavior
@fluentui/reactsupports rendering in shadow DOM.Apologies in advance for the large PR, I know we strive to avoid those for a variety of reasons but given the extent of this change I didn't feel it was feasible to introduce it piecemeal. I have been collecting all the changes in the
shadow-dombranch which I've kept up-to-date withmasterand the PRs intoshadow-domhave been reviewed so this is not the first time this code is being looked at.Related Issue(s)