Skip to content

Commit faf6311

Browse files
authored
fix(react-context-selector): avoid useState eager-bailout pitfall (#36002)
1 parent 13bcf75 commit faf6311

9 files changed

Lines changed: 157 additions & 101 deletions
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix: rewrite useContextSelector to avoid React's eager-bailout pitfall on memoized consumers",
4+
"packageName": "@fluentui/react-context-selector",
5+
"email": "olfedias@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

packages/react-components/react-context-selector/etc/react-context-selector.api.md

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,18 @@ export type ContextSelector<Value, SelectedValue> = (value: Value) => SelectedVa
1717

1818
// @internal (undocumented)
1919
export type ContextValue<Value> = {
20-
listeners: ((payload: readonly [ContextVersion, Value]) => void)[];
21-
value: React_2.MutableRefObject<Value>;
22-
version: React_2.MutableRefObject<ContextVersion>;
20+
listeners: ((payload: Value) => void)[];
21+
value: {
22+
current: Value;
23+
};
24+
isDefault?: boolean;
2325
};
2426

25-
// @internal (undocumented)
26-
export type ContextValues<Value> = ContextValue<Value> & {
27-
listeners: ((payload: readonly [ContextVersion, Record<string, Value>]) => void)[];
28-
};
29-
30-
// @internal (undocumented)
31-
export type ContextVersion = number;
32-
3327
// @internal (undocumented)
3428
export const createContext: <Value>(defaultValue: Value) => Context<Value>;
3529

3630
// @internal
37-
export const useContextSelector: <Value, SelectedValue>(context: Context<Value>, selector: ContextSelector<Value, SelectedValue>) => SelectedValue;
31+
export const useContextSelector: <Value, SelectedValue>(context: Context<Value>, selectorFn: ContextSelector<Value, SelectedValue>) => SelectedValue;
3832

3933
// @internal
4034
export function useHasParentContext<Value>(context: Context<Value>): boolean;
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createContext } from './createContext';
2-
import * as ReactIs from 'react-is';
2+
import { isValidElementType } from 'react-is';
33

44
describe('createContext', () => {
55
it('creates a Provider component', () => {
66
const Context = createContext(null);
7-
expect(ReactIs.isValidElementType(Context.Provider)).toBeTruthy();
7+
8+
expect(isValidElementType(Context.Provider)).toBeTruthy();
89
});
910
});

packages/react-components/react-context-selector/src/createContext.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,23 @@ const createProvider = <Value>(Original: React.Provider<ContextValue<Value>>) =>
1010
const Provider: React.FC<React.ProviderProps<Value>> = props => {
1111
// Holds an actual "props.value"
1212
const valueRef = React.useRef(props.value);
13-
// Used to sync context updates and avoid stale values, can be considered as render/effect counter of Provider.
14-
const versionRef = React.useRef(0);
1513

1614
// A stable object, is used to avoid context updates via mutation of its values.
1715
const contextValue = React.useRef<ContextValue<Value>>(null);
1816

1917
if (!contextValue.current) {
2018
contextValue.current = {
2119
value: valueRef,
22-
version: versionRef,
2320
listeners: [],
2421
};
2522
}
2623

2724
useIsomorphicLayoutEffect(() => {
2825
valueRef.current = props.value;
29-
versionRef.current += 1;
3026

3127
runWithPriority(NormalPriority, () => {
3228
(contextValue.current as ContextValue<Value>).listeners.forEach(listener => {
33-
listener([versionRef.current, props.value]);
29+
listener(props.value);
3430
});
3531
});
3632
}, [props.value]);
@@ -53,8 +49,8 @@ export const createContext = <Value>(defaultValue: Value): Context<Value> => {
5349
// eslint-disable-next-line @fluentui/no-context-default-value
5450
const context = React.createContext<ContextValue<Value>>({
5551
value: { current: defaultValue },
56-
version: { current: -1 },
5752
listeners: [],
53+
isDefault: true,
5854
});
5955

6056
context.Provider = createProvider<Value>(context.Provider);

packages/react-components/react-context-selector/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ export { createContext } from './createContext';
22
export { useContextSelector } from './useContextSelector';
33
export { useHasParentContext } from './useHasParentContext';
44
// eslint-disable-next-line @fluentui/ban-context-export
5-
export type { Context, ContextSelector, ContextValue, ContextValues, ContextVersion } from './types';
5+
export type { Context, ContextSelector, ContextValue } from './types';

packages/react-components/react-context-selector/src/types.ts

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,16 @@ export type Context<Value> = React.Context<Value> & {
1010

1111
export type ContextSelector<Value, SelectedValue> = (value: Value) => SelectedValue;
1212

13-
/**
14-
* @internal
15-
*/
16-
export type ContextVersion = number;
17-
1813
/**
1914
* @internal
2015
*/
2116
export type ContextValue<Value> = {
2217
/** Holds a set of subscribers from components. */
23-
listeners: ((payload: readonly [ContextVersion, Value]) => void)[];
18+
listeners: ((payload: Value) => void)[];
2419

2520
/** Holds an actual value of React's context that will be propagated down for computations. */
26-
value: // eslint-disable-next-line @typescript-eslint/no-deprecated
27-
React.MutableRefObject<Value>;
21+
value: { current: Value };
2822

29-
/** A version field is used to sync a context value and consumers. */
30-
version: // eslint-disable-next-line @typescript-eslint/no-deprecated
31-
React.MutableRefObject<ContextVersion>;
32-
};
33-
34-
/**
35-
* @internal
36-
*/
37-
export type ContextValues<Value> = ContextValue<Value> & {
38-
/** List of listners to publish changes */
39-
listeners: ((payload: readonly [ContextVersion, Record<string, Value>]) => void)[];
23+
/** Indicates if a context holds default value. */
24+
isDefault?: boolean;
4025
};

packages/react-components/react-context-selector/src/useContextSelector.test.tsx

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,99 @@ describe('useContextSelector', () => {
9898
expect(document.querySelector<HTMLElement>('.test-component')?.dataset.active).toBe('true');
9999
expect(onUpdate).toHaveBeenCalledTimes(2);
100100
});
101+
102+
it('memoized consumers re-render only when their selected slice changes', () => {
103+
const MemoizedTestComponent = React.memo(TestComponent);
104+
105+
const onUpdates: Record<number, jest.Mock> = {
106+
1: jest.fn(),
107+
2: jest.fn(),
108+
3: jest.fn(),
109+
4: jest.fn(),
110+
};
111+
112+
render(
113+
<TestProvider>
114+
<MemoizedTestComponent index={1} onUpdate={onUpdates[1]} />
115+
<MemoizedTestComponent index={2} onUpdate={onUpdates[2]} />
116+
<MemoizedTestComponent index={3} onUpdate={onUpdates[3]} />
117+
<MemoizedTestComponent index={4} onUpdate={onUpdates[4]} />
118+
</TestProvider>,
119+
{ container: container as HTMLElement },
120+
);
121+
122+
// Initial render. TestProvider starts at index=0, so none are active. Each memoized item has committed once — the
123+
// `onUpdate` effect fires once per item on mount.
124+
Object.values(onUpdates).forEach(m => expect(m).toHaveBeenCalledTimes(1));
125+
126+
// Click 1: 0 → 1. Only index=1 flips (active: false → true).
127+
jest.clearAllMocks();
128+
act(() => {
129+
document.querySelector<HTMLElement>('.test-provider')?.click();
130+
});
131+
132+
expect(onUpdates[1]).toHaveBeenCalledTimes(1); // true => false
133+
expect(onUpdates[2]).toHaveBeenCalledTimes(0);
134+
expect(onUpdates[3]).toHaveBeenCalledTimes(0);
135+
expect(onUpdates[4]).toHaveBeenCalledTimes(0);
136+
137+
// Click 2: 1 → 2. index=1 flips (active: true → false), index=2 flips (active: false → true).
138+
// Items 3 and 4 did not change and must not re-render.
139+
jest.clearAllMocks();
140+
act(() => {
141+
document.querySelector<HTMLElement>('.test-provider')?.click();
142+
});
143+
144+
expect(onUpdates[1]).toHaveBeenCalledTimes(1); // true => false
145+
expect(onUpdates[2]).toHaveBeenCalledTimes(1); // false => true
146+
expect(onUpdates[3]).toHaveBeenCalledTimes(0); // ← was 2 under the old eager-bailout with `useState()`
147+
expect(onUpdates[4]).toHaveBeenCalledTimes(0);
148+
149+
// Click 3: 2 → 3.
150+
jest.clearAllMocks();
151+
act(() => {
152+
document.querySelector<HTMLElement>('.test-provider')?.click();
153+
});
154+
155+
expect(onUpdates[1]).toHaveBeenCalledTimes(0);
156+
expect(onUpdates[2]).toHaveBeenCalledTimes(1); // true => false
157+
expect(onUpdates[3]).toHaveBeenCalledTimes(1); // false => true
158+
expect(onUpdates[4]).toHaveBeenCalledTimes(0);
159+
});
160+
161+
it('a single consumer throw does not starve later subscribers of context updates', () => {
162+
const ThrowingConsumer: React.FC<{ threshold: number }> = ({ threshold }) => {
163+
useContextSelector(TestContext, v => {
164+
if (v.index > threshold) {
165+
throw new Error('selector cannot handle this value');
166+
}
167+
return v.index;
168+
});
169+
170+
return null;
171+
};
172+
173+
const onUpdate = jest.fn();
174+
175+
render(
176+
<TestProvider>
177+
<ThrowingConsumer threshold={0} />
178+
<TestComponent index={1} onUpdate={onUpdate} />
179+
</TestProvider>,
180+
{ container: container as HTMLElement },
181+
);
182+
183+
expect(onUpdate).toHaveBeenCalledTimes(1);
184+
185+
// Click: index goes 0 → 1. The first consumer's selector will throw
186+
// (1 > 0). The downstream TestComponent must still receive the update
187+
// because listener error isolation prevents `listeners.forEach` from
188+
// short-circuiting.
189+
act(() => {
190+
document.querySelector<HTMLElement>('.test-provider')?.click();
191+
});
192+
193+
expect(document.querySelector<HTMLElement>('.test-component')?.dataset.active).toBe('true');
194+
expect(onUpdate).toHaveBeenCalledTimes(2);
195+
});
101196
});
Lines changed: 38 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
'use client';
22

3-
import { useEventCallback, useIsomorphicLayoutEffect } from '@fluentui/react-utilities';
3+
import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities';
44
import * as React from 'react';
55

6-
import type { Context, ContextSelector, ContextValue, ContextVersion } from './types';
6+
import type { Context, ContextSelector, ContextValue } from './types';
77

88
/**
99
* This hook returns context selected value by selector.
@@ -14,74 +14,56 @@ import type { Context, ContextSelector, ContextValue, ContextVersion } from './t
1414
*/
1515
export const useContextSelector = <Value, SelectedValue>(
1616
context: Context<Value>,
17-
selector: ContextSelector<Value, SelectedValue>,
17+
selectorFn: ContextSelector<Value, SelectedValue>,
1818
): SelectedValue => {
1919
const contextValue = React.useContext(context as unknown as Context<ContextValue<Value>>);
20+
const { value: valueRef, listeners } = contextValue;
2021

21-
const {
22-
value: { current: value },
23-
version: { current: version },
24-
listeners,
25-
} = contextValue;
26-
const selected = selector(value);
22+
// Read valueRef during render and return selector(value) directly. This is analogous to `useSyncExternalStore`'s
23+
// `getSnapshot` and is the only way to select a slice from a shared ref-based store without re-rendering every
24+
// consumer on every provider update.
25+
const valueAtRender = selectorFn(valueRef.current);
26+
const [, forceUpdate] = React.useReducer((x: number) => x + 1, 0);
2727

28-
const [state, setState] = React.useState<readonly [Value, SelectedValue]>([value, selected]);
29-
const dispatch = (
30-
payload:
31-
| undefined // undefined from render below
32-
| readonly [ContextVersion, Value], // from provider effect
33-
) => {
34-
setState(prevState => {
35-
if (!payload) {
36-
// early bail out when is dispatched during render
37-
return [value, selected] as const;
38-
}
39-
40-
if (payload[0] <= version) {
41-
if (Object.is(prevState[1], selected)) {
42-
return prevState; // bail out
43-
}
28+
// Refs holding the current selector and the most-recently-returned slice.
29+
// Updated in a layout effect (ordering: children first, then provider) so
30+
// they are current by the time the provider's listener loop fires.
31+
const selectorFnRef = React.useRef<ContextSelector<Value, SelectedValue>>(selectorFn);
32+
const lastValueAtRender = React.useRef<SelectedValue>(valueAtRender);
4433

45-
return [value, selected] as const;
46-
}
34+
useIsomorphicLayoutEffect(() => {
35+
selectorFnRef.current = selectorFn;
36+
lastValueAtRender.current = valueAtRender;
37+
});
4738

39+
useIsomorphicLayoutEffect(() => {
40+
const listener = (payload: Value) => {
41+
// Selectors can throw on transiently-inconsistent inputs (stale props vs. newer context value). Swallow so a
42+
// single consumer's throw doesn't abort the provider's `listeners.forEach`.
4843
try {
49-
if (Object.is(prevState[0], payload[1])) {
50-
return prevState; // do not update
51-
}
52-
53-
const nextSelected = selector(payload[1]);
44+
const nextSelectedValue = selectorFnRef.current(payload);
5445

55-
if (Object.is(prevState[1], nextSelected)) {
56-
return prevState; // do not update
46+
if (!Object.is(lastValueAtRender.current, nextSelectedValue)) {
47+
forceUpdate();
5748
}
58-
59-
return [payload[1], nextSelected] as const;
60-
} catch (e) {
61-
// ignored (stale props or some other reason)
49+
} catch {
50+
// ignored (stale props or similar — heals on the next parent-driven render)
6251
}
52+
};
6353

64-
// explicitly spread to enforce typing
65-
return [prevState[0], prevState[1]] as const; // schedule update
66-
});
67-
};
68-
69-
if (!Object.is(state[1], selected)) {
70-
// schedule re-render
71-
// this is safe because it's self contained
72-
dispatch(undefined);
73-
}
54+
listeners.push(listener);
7455

75-
const stableDispatch = useEventCallback(dispatch);
76-
77-
useIsomorphicLayoutEffect(() => {
78-
listeners.push(stableDispatch);
56+
// Effect-fixup: catch updates that occurred between render and effect run (Relay's useFragmentInternal pattern).
57+
listener(valueRef.current);
7958

8059
return () => {
81-
const index = listeners.indexOf(stableDispatch);
82-
listeners.splice(index, 1);
60+
const index = listeners.indexOf(listener);
61+
62+
if (index !== -1) {
63+
listeners.splice(index, 1);
64+
}
8365
};
84-
}, [stableDispatch, listeners]);
66+
}, [listeners, valueRef]);
8567

86-
return state[1] as SelectedValue;
68+
return valueAtRender;
8769
};

packages/react-components/react-context-selector/src/useHasParentContext.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,5 @@ import type { Context, ContextValue } from './types';
1414
export function useHasParentContext<Value>(context: Context<Value>): boolean {
1515
const contextValue = React.useContext(context as unknown as Context<ContextValue<Value>>);
1616

17-
if (contextValue.version) {
18-
return contextValue.version.current !== -1;
19-
}
20-
21-
return false;
17+
return !contextValue.isDefault;
2218
}

0 commit comments

Comments
 (0)