feat: Add enableScopedSelectors prop to Stack that, when true, makes the Stack styles selectors be more scoped to not be as expensive in style recalculation by khmakoto · Pull Request #25397 · microsoft/fluentui

khmakoto · 2022-10-27T00:38:17Z

Previous Behavior

The Stack component was very expensive for style recalculation performance because children selectors are not scoped (they are of the type > * and the browser evaluates selectors from right to left so it always matches everything in the page first). Some improvements were made in #25381, but the performance of the component was still not up to the level we wanted it to be.

Here are the results of a trace running on our v8 docsite's Stack page, where most of the most expensive selectors are Stack-related:

rulesFastRejected: 47342
rulesRejected: 5959
elapsed time: 5711 microseconds

New Behavior

This PR adds an enableScopedSelectors prop that, when set to true, makes the Stack children selectors be more scoped (they go from being of the type > * to being of the type > .ms-Stack-child). This greatly reduces the style recalculation cost.

We have gated this behavior behind a prop because this approach requires all immediate children of the Stack component to be able to receive and apply a className prop (excluding Fragments, in which case all their immediate children are the ones looked at instead). Since this was not a requirement before, this could create some issues for people who are already using the Stack component in their product, which would be a breaking change. By making this behavior opt-in, we can still provide a way for people to get the performance benefits without having a breaking change in the library.

Enabling this change for all Stacks in the examples of our v8 docsite's Stack page gives us the following numbers, which are much improved from before:

rulesFastRejected: 8281
rulesRejected: 5444
elapsed time: 2474 microseconds

Furthermore, the v8 docsite leverages the Stack component in several places that are not the examples. I experimented by enabling this change in the whole page (this change is not included in this PR and was just for experimentation) and I got much better results than even the ones above:

rulesFastRejected: 5288
rulesRejected: 4983
elapsed time: 672 microseconds

Related Issue(s)

Fixes #24259
Follow-up of #25381

…the Stack styles selectors be more scoped to not be as expensive in style recalculation.

…is set to true.

size-auditor · 2022-10-27T00:50:01Z

Asset size changes

Project	Bundle	Baseline Size	New Size	Difference
office-ui-fabric-react	fluentui-react-Stack	39.335 kB	40.244 kB	909 bytes
office-ui-fabric-react	fluentui-react-TeachingBubble	193.143 kB	193.751 kB	608 bytes

Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B

Baseline commit: fe895d6bb4f9df0304f63f7e77acafef4b912925 (build)

fabricteam · 2022-10-27T00:51:47Z

📊 Bundle size report

_{🤖 This report was generated against fe895d6bb4f9df0304f63f7e77acafef4b912925}

codesandbox-ci · 2022-10-27T00:51:51Z

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

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

fabricteam · 2022-10-27T01:20:07Z

Perf Analysis (`@fluentui/react`)

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
StackWithIntrinsicChildren	mount	2761	2968	5000	Possible regression
StackWithTextChildren	mount	5695	5872	5000	Possible regression

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
BaseButton	mount	1402	1430	5000
Breadcrumb	mount	3615	3488	1000
Checkbox	mount	3164	3181	5000
CheckboxBase	mount	2776	2908	5000
ChoiceGroup	mount	5270	5372	5000
ComboBox	mount	1467	1483	1000
CommandBar	mount	11496	11475	1000
ContextualMenu	mount	13075	13013	1000
DefaultButton	mount	1639	1650	5000
DetailsRow	mount	4316	4361	5000
DetailsRowFast	mount	4375	4308	5000
DetailsRowNoStyles	mount	4191	4243	5000
Dialog	mount	3613	3637	1000
DocumentCardTitle	mount	700	717	1000
Dropdown	mount	4224	3834	5000
FocusTrapZone	mount	2425	2473	5000
FocusZone	mount	2346	2349	5000
GroupedList	mount	62664	70674	2
GroupedList	virtual-rerender	29610	29789	2
GroupedList	virtual-rerender-with-unmount	108278	110366	2
GroupedListV2	mount	676	683	2
GroupedListV2	virtual-rerender	661	670	2
GroupedListV2	virtual-rerender-with-unmount	663	684	2
IconButton	mount	2309	2303	5000
Label	mount	870	875	5000
Layer	mount	5146	5102	5000
Link	mount	1033	992	5000
MenuButton	mount	1990	2017	5000
MessageBar	mount	2803	2773	5000
Nav	mount	3966	3872	1000
OverflowSet	mount	1664	1618	5000
Panel	mount	3047	3001	1000
Persona	mount	1530	1536	1000
Pivot	mount	2006	1923	1000
PrimaryButton	mount	1798	1816	5000
Rating	mount	8299	8305	5000
SearchBox	mount	1850	1796	5000
Shimmer	mount	3358	3331	5000
Slider	mount	2455	2514	5000
SpinButton	mount	5639	5512	5000
Spinner	mount	979	943	5000
SplitButton	mount	3696	3716	5000
Stack	mount	989	1011	5000
StackWithIntrinsicChildren	mount	2761	2968	5000	Possible regression
StackWithTextChildren	mount	5695	5872	5000	Possible regression
SwatchColorPicker	mount	12366	12420	5000
TagPicker	mount	3074	3140	5000
TeachingBubble	mount	100640	99561	5000
Text	mount	958	944	5000
TextField	mount	1906	1864	5000
ThemeProvider	mount	1819	1847	5000
ThemeProvider	virtual-rerender	1280	1291	5000
ThemeProvider	virtual-rerender-with-unmount	2566	2590	5000
Toggle	mount	1326	1319	5000
buttonNative	mount	644	658	5000

packages/react/src/components/Stack/Stack.styles.ts

jspurlin

…tackScopedSelectors

sopranopillow · 2022-10-27T23:26:31Z

packages/react/src/components/Stack/Stack.types.ts

+
+  /**
+   * Defines if scoped style selectors are enabled for the Stack component, which greatly helps in style recalculation
+   * performance but requires children of the Stack to be able to accept a className prop (excluding Fragments).


nit:

Suggested change

* performance but requires children of the Stack to be able to accept a className prop (excluding Fragments).

* performance, but requires children of the Stack to be able to accept a className prop (excluding Fragments).

behowell · 2022-10-27T23:31:13Z

packages/react/src/components/Stack/Stack.styles.ts

  const disableShrinkStyles = {
    // flexShrink styles are applied by the StackItem
-    '> *:not(.ms-StackItem)': {
+    [`> ${enableScopedSelectors ? '.' + GlobalClassNames.child : '*'}:not(.${StackItemGlobalClassNames.root})`]: {


Maybe save the scoped selector to a local const, since you use it in multiple places. It would also make this easier to read.

const childSelector = enableScopedSelectors ? `> .${GlobalClassNames.child}` : '> *';

Suggested change

[`> ${enableScopedSelectors ? '.' + GlobalClassNames.child : '*'}:not(.${StackItemGlobalClassNames.root})`]: {

[`${childSelector}:not(.${StackItemGlobalClassNames.root})`]: {

behowell · 2022-10-27T23:32:15Z

packages/react/src/components/Stack/Stack.styles.ts

          maxWidth: '100vw',

-          '> *': {
+          [`> ${enableScopedSelectors ? '.' + GlobalClassNames.child : '*'}`]: {


If you use my childSelector suggestion, this would be:

Suggested change

[`> ${enableScopedSelectors ? '.' + GlobalClassNames.child : '*'}`]: {

[childSelector]: {

Ditto below.

behowell · 2022-10-27T23:37:27Z

packages/react/src/components/Stack/Stack.tsx

+        : null;
+    }
+
+    const childAsReactElement = child as React.ReactElement;


(nit) is this cast necessary? I think the React.isValidElement check above should narrow the type of child to React.ReactElement.

* master: (22 commits) fix(react-menu): removes exposing of internal type FluentTriggerComponent (microsoft#25410) fix(react-popover): removes exposing of internal type FluentTriggerComponent (microsoft#25411) applying package updates fix(react-tooltip): removes exposing of internal type FluentTriggerComponent (microsoft#25409) chore: Reducing bundle size of Stack by moving selector used in multiple places to local const (microsoft#25429) docs(rfcs): Simple component implementation (microsoft#25139) Fix migration publishing (microsoft#25422) Integrate storywright for story tests - As part of exploring screener alternative (microsoft#25399) fix(react-utilities): exposes internal methods used in API surface (microsoft#25406) fix(react-dialog): removes exposing of internal type FluentTriggerComponent (microsoft#25408) fix(react-context-selector): exposes internal type ContextSelector (microsoft#25404) fix(react-aria): exposes internal leaking types (microsoft#25403) fix(react-shared-contexts): exposes internal leaks used in the API surface (microsoft#25405) fix(react-positioning): exposes new typings to avoid exposing internal methods (microsoft#25407) applying package updates fix: Allowing DatePicker to be focusable within FocusZones by default (microsoft#25428) fix: Pad in slider so the thumb does not render outside the bounds of the root element (microsoft#25378) feat: Add enableScopedSelectors prop to Stack that, when true, makes the Stack styles selectors be more scoped to not be as expensive in style recalculation (microsoft#25397) fix(react-avatar): Remove gaps between AvatarGroupItem/OveflowButton and its outline (microsoft#25382) fix: Combobox text attribute ignored when empty string is passed (microsoft#24665) ...

…the Stack styles selectors be more scoped to not be as expensive in style recalculation (microsoft#25397) * feat: Add enableScopedSelectors prop to Stack that, when true, makes the Stack styles selectors be more scoped to not be as expensive in style recalculation. * Adding change file. * Only apply stack child classname to children if enableScopeSelectors is set to true. * Addressing PR feedback. * Small improvement. Co-authored-by: KHMakoto <humberto_makoto@hotmail.com>

KHMakoto added 2 commits October 26, 2022 17:19

feat: Add enableScopedSelectors prop to Stack that, when true, makes …

81e5ad9

…the Stack styles selectors be more scoped to not be as expensive in style recalculation.

Adding change file.

2b20519

khmakoto added Component: Stack Area: Performance Fluent UI react (v8) Issues about @fluentui/react (v8) labels Oct 27, 2022

khmakoto requested a review from a team as a code owner October 27, 2022 00:38

khmakoto self-assigned this Oct 27, 2022

khmakoto requested a review from a team as a code owner October 27, 2022 00:38

msft-fluent-ui-bot added the PR: API Modified label Oct 27, 2022

github-actions bot added this to the October Project Cycle Q4 2022 milestone Oct 27, 2022

Only apply stack child classname to children if enableScopeSelectors …

1f6f98e

…is set to true.

spmonahan reviewed Oct 27, 2022

View reviewed changes

packages/react/src/components/Stack/Stack.styles.ts Show resolved Hide resolved

packages/react/src/components/Stack/Stack.styles.ts Outdated Show resolved Hide resolved

KHMakoto added 2 commits October 27, 2022 12:01

Addressing PR feedback.

a577651

Small improvement.

cf05ee7

jspurlin approved these changes Oct 27, 2022

View reviewed changes

Merge branch 'master' of https://github.com/microsoft/fluentui into s…

2423974

…tackScopedSelectors

spmonahan approved these changes Oct 27, 2022

View reviewed changes

khmakoto merged commit 277d440 into microsoft:master Oct 27, 2022

khmakoto deleted the stackScopedSelectors branch October 27, 2022 23:26

sopranopillow approved these changes Oct 27, 2022

View reviewed changes

behowell approved these changes Oct 27, 2022

View reviewed changes

khmakoto mentioned this pull request Oct 28, 2022

chore: Reducing bundle size of Stack by moving selector used in multiple places to local const #25429

Merged

khmakoto mentioned this pull request Nov 15, 2022

fix: Making Stack properly render non-ReactElement children #25685

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add enableScopedSelectors prop to Stack that, when true, makes the Stack styles selectors be more scoped to not be as expensive in style recalculation#25397

feat: Add enableScopedSelectors prop to Stack that, when true, makes the Stack styles selectors be more scoped to not be as expensive in style recalculation#25397
khmakoto merged 6 commits intomicrosoft:masterfrom
khmakoto:stackScopedSelectors

khmakoto commented Oct 27, 2022

Uh oh!

size-auditor bot commented Oct 27, 2022 •

edited

Loading

Uh oh!

fabricteam commented Oct 27, 2022 •

edited

Loading

Uh oh!

codesandbox-ci bot commented Oct 27, 2022 •

edited

Loading

Uh oh!

fabricteam commented Oct 27, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

jspurlin left a comment

Uh oh!

sopranopillow Oct 27, 2022

Uh oh!

behowell Oct 27, 2022

Uh oh!

behowell Oct 27, 2022

Uh oh!

behowell Oct 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

	* performance but requires children of the Stack to be able to accept a className prop (excluding Fragments).
	* performance, but requires children of the Stack to be able to accept a className prop (excluding Fragments).

	[`> ${enableScopedSelectors ? '.' + GlobalClassNames.child : '*'}:not(.${StackItemGlobalClassNames.root})`]: {
	[`${childSelector}:not(.${StackItemGlobalClassNames.root})`]: {

	[`> ${enableScopedSelectors ? '.' + GlobalClassNames.child : '*'}`]: {
	[childSelector]: {

Conversation

khmakoto commented Oct 27, 2022

Previous Behavior

New Behavior

Related Issue(s)

Uh oh!

size-auditor bot commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

codesandbox-ci bot commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabricteam commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react)

Uh oh!

Uh oh!

Uh oh!

jspurlin left a comment

Choose a reason for hiding this comment

Uh oh!

sopranopillow Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

behowell Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

behowell Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

behowell Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

size-auditor bot commented Oct 27, 2022 •

edited

Loading

fabricteam commented Oct 27, 2022 •

edited

Loading

codesandbox-ci bot commented Oct 27, 2022 •

edited

Loading

fabricteam commented Oct 27, 2022 •

edited

Loading

Perf Analysis (`@fluentui/react`)