Skip to content

fix: useContextSelector with React 18 #30951

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

Merged

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Apr 3, 2024

Previous Behavior

useContextSelector() implementation relies on bail-outs if values are shallow equal:

if (objectIs(prevState[1], selected)) {
return prevState; // bail out
}

However, this does not work anymore with React 18 💥

React 17

// Parent: render
// Child: render
// Parent: useLayoutEffect (note 1)
// Child: bail-out ✅ 

React 18

// Parent: render
// Child: render
// Parent: useLayoutEffect (note 1)
// Child: re-render 💣 
Note 1: useLayoutEffect

This effect propagates updates to its consumers once "Parent" gets rendered:

useIsomorphicLayoutEffect(() => {
valueRef.current = props.value;
versionRef.current += 1;
runWithPriority(NormalPriority, () => {
(contextValue.current as ContextValue<Value>).listeners.forEach(listener => {
listener([versionRef.current, props.value]);
});
});
}, [props.value]);

This results in performance degradation during the initial render 🐌 The change in the behavior was not expected, but it's a part of React 18: facebook/react#22445.

New Behavior

This PR switches useReducer() used in useContextSelector() to useState() that still has the behavior with bailout. In the longterm, we should explore the approach of using useSyncExternalStore() for this functionality.

@github-actions github-actions bot added this to the April Project Cycle Q1 2024 milestone Apr 3, 2024
@fabricteam
Copy link
Collaborator

fabricteam commented Apr 3, 2024

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 650 627 5000
Button mount 311 321 5000
Field mount 1123 1158 5000
FluentProvider mount 748 722 5000
FluentProviderWithTheme mount 80 82 10
FluentProviderWithTheme virtual-rerender 63 64 10
FluentProviderWithTheme virtual-rerender-with-unmount 73 78 10
MakeStyles mount 891 895 50000
Persona mount 1774 1800 5000
SpinButton mount 1424 1413 5000

Copy link

codesandbox-ci bot commented Apr 3, 2024

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.

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 3, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-accordion
Accordion (including children components)
99.599 kB
30.264 kB
99.547 kB
30.228 kB
-52 B
-36 B
react-avatar
AvatarGroupItem
64.829 kB
20.272 kB
64.958 kB
20.32 kB
129 B
48 B
react-combobox
Combobox (including child components)
102.735 kB
33.197 kB
102.675 kB
33.177 kB
-60 B
-20 B
react-combobox
Dropdown (including child components)
104.06 kB
33.101 kB
104 kB
33.077 kB
-60 B
-24 B
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
219.521 kB
62.081 kB
219.456 kB
62.061 kB
-65 B
-20 B
react-dialog
Dialog (including children components)
100.928 kB
29.921 kB
100.874 kB
29.9 kB
-54 B
-21 B
react-infobutton
InfoButton
138.694 kB
43.395 kB
138.64 kB
43.375 kB
-54 B
-20 B
react-infobutton
InfoLabel
142.495 kB
44.625 kB
142.442 kB
44.604 kB
-53 B
-21 B
react-menu
Menu (including children components)
152.268 kB
45.708 kB
152.208 kB
45.678 kB
-60 B
-30 B
react-menu
Menu (including selectable components)
154.954 kB
46.274 kB
154.894 kB
46.242 kB
-60 B
-32 B
react-overflow
hooks only
12.86 kB
4.825 kB
12.813 kB
4.81 kB
-47 B
-15 B
react-popover
Popover
126.884 kB
39.803 kB
126.83 kB
39.78 kB
-54 B
-23 B
react-swatch-picker-preview
@fluentui/react-swatch-picker-preview - package
103.385 kB
29.763 kB
103.331 kB
29.745 kB
-54 B
-18 B
react-table
DataGrid
165.647 kB
46.063 kB
165.596 kB
46.024 kB
-51 B
-39 B
react-timepicker-compat
TimePicker
104.756 kB
34.581 kB
104.696 kB
34.558 kB
-60 B
-23 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
global-context
createContext
510 B
328 B
global-context
createContextSelector
537 B
339 B
react-alert
Alert
83.737 kB
23.475 kB
react-avatar
Avatar
50.175 kB
15.944 kB
react-avatar
AvatarGroup
19.702 kB
7.794 kB
react-checkbox
Checkbox
35.656 kB
12.07 kB
react-components
react-components: Button, FluentProvider & webLightTheme
71.098 kB
20.515 kB
react-components
react-components: FluentProvider & webLightTheme
43.585 kB
14.352 kB
react-datepicker-compat
DatePicker Compat
225.764 kB
63.187 kB
react-field
Field
22.976 kB
8.722 kB
react-input
Input
28.122 kB
9.36 kB
react-persona
Persona
57.066 kB
17.821 kB
react-portal-compat
PortalCompatProvider
7.944 kB
2.588 kB
react-progress
ProgressBar
17.428 kB
6.899 kB
react-radio
Radio
32.95 kB
10.252 kB
react-radio
RadioGroup
15.354 kB
6.265 kB
react-select
Select
28.609 kB
10.204 kB
react-slider
Slider
39.949 kB
12.968 kB
react-spinbutton
SpinButton
36.774 kB
11.789 kB
react-switch
Switch
35.14 kB
11.199 kB
react-table
Table (Primitives only)
45.324 kB
14.116 kB
react-table
Table as DataGrid
136.573 kB
36.817 kB
react-table
Table (Selection only)
76.334 kB
20.553 kB
react-table
Table (Sort only)
74.977 kB
20.155 kB
react-tags
InteractionTag
15.284 kB
6.07 kB
react-tags
Tag
30.029 kB
9.461 kB
react-tags
TagGroup
80.68 kB
24.073 kB
react-textarea
Textarea
30.947 kB
10.477 kB
🤖 This report was generated against fd38bbcbb31e230c19a9c552fa03017945089b9d

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 3, 2024

🕵 fluentuiv9 No visual regressions between this PR and main

@layershifter layershifter force-pushed the fix/context-selector-attemp2 branch from 4182e5a to 85dd57c Compare April 3, 2024 15:47
@layershifter layershifter marked this pull request as ready for review June 12, 2024 08:17
@layershifter layershifter requested a review from a team as a code owner June 12, 2024 08:17
@layershifter layershifter merged commit 9494a77 into microsoft:master Jun 13, 2024
@layershifter layershifter deleted the fix/context-selector-attemp2 branch June 13, 2024 11:37
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants