From 1e84b3ee61caeec534fa51f7ac9089bd3cab9f4a Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Sat, 18 Apr 2026 11:14:17 +0200 Subject: [PATCH 1/5] fix(react-context-selector): rewrite useContextSelector to avoid eager-bailout pitfall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous implementation depends on React's useState eager-bailout fast path in dispatchSetStateInternal: if (fiber.lanes === NoLanes && (alternate === null || alternate.lanes === NoLanes)) { // eager bailout — drop the update without scheduling a render } The fiber passed is the one bound at mount. After the first listener-driven render commits, that fiber becomes the alternate of the new current — but its .lanes is never cleared from the enqueueUpdate that preceded the render. From that point on, every subsequent listener dispatch fails the NoLanes precondition. Updates are queued normally, schedule a render, and the reducer runs in-render only to return prevState. React then calls bailoutOnAlreadyFinishedWork — the DOM doesn't update, but the component function already executed. This is the "Glitchy Behavior" documented in the context-selector-tearing RFC. New implementation: - Reads valueRef.current during render (like useSyncExternalStore's getSnapshot). - No [value, selected] state tuple. No setState(prev => prev) trick. - useReducer(x => x + 1) as opaque force-update counter. - Listener compares selector(newValue) against lastReturnedRef and forces only when the slice actually differs. - Layout-effect fixup per Relay's useFragmentInternal pattern. Preserves: public API, listener payload shape ([version, value]), useHasParentContext behavior, Level 1 tearing behavior. See RFC PR for the full Option D writeup. --- ...-3a434472-dd03-454e-82d2-431c6a6b7366.json | 7 ++ .../src/useContextSelector.ts | 107 ++++++++---------- 2 files changed, 54 insertions(+), 60 deletions(-) create mode 100644 change/@fluentui-react-context-selector-3a434472-dd03-454e-82d2-431c6a6b7366.json diff --git a/change/@fluentui-react-context-selector-3a434472-dd03-454e-82d2-431c6a6b7366.json b/change/@fluentui-react-context-selector-3a434472-dd03-454e-82d2-431c6a6b7366.json new file mode 100644 index 00000000000000..cd2abaa7f448cb --- /dev/null +++ b/change/@fluentui-react-context-selector-3a434472-dd03-454e-82d2-431c6a6b7366.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: rewrite useContextSelector to avoid React's eager-bailout pitfall on memoized consumers", + "packageName": "@fluentui/react-context-selector", + "email": "olfedias@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-context-selector/src/useContextSelector.ts b/packages/react-components/react-context-selector/src/useContextSelector.ts index bbd90e44a8c519..ba6ee59af9f0c0 100644 --- a/packages/react-components/react-context-selector/src/useContextSelector.ts +++ b/packages/react-components/react-context-selector/src/useContextSelector.ts @@ -1,6 +1,6 @@ 'use client'; -import { useEventCallback, useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; +import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; import * as React from 'react'; import type { Context, ContextSelector, ContextValue, ContextVersion } from './types'; @@ -17,71 +17,58 @@ export const useContextSelector = ( selector: ContextSelector, ): SelectedValue => { const contextValue = React.useContext(context as unknown as Context>); + const { value: valueRef, listeners } = contextValue; + + // Read valueRef during render and return selector(value) directly. This is + // analogous to useSyncExternalStore's getSnapshot and is the only way to + // select a slice from a shared ref-based store without re-rendering every + // consumer on every provider update. + const selectedAtRender = selector(valueRef.current); + + // Opaque force-update. Kept deliberately separate from a `[value, selected]` + // useState tuple: the previous implementation relied on React's eager-bailout + // fast path (`setState(prev => prev)` → `Object.is(eagerState, currentState)`) + // which silently stops firing after any listener-driven render commits — the + // fiber bound at mount becomes the stale alternate and its `fiber.lanes` + // retains SyncLane, permanently failing the `NoLanes` precondition of + // `dispatchSetStateInternal`. See RFC addendum "Option D" for details. + const [, forceUpdate] = React.useReducer((x: number) => x + 1, 0); + + // Refs holding the current selector and the most-recently-returned slice. + // Updated in a layout effect (ordering: children first, then provider) so + // they are current by the time the provider's listener loop fires. + const selectorRef = React.useRef>(selector); + const lastReturnedRef = React.useRef(selectedAtRender); - const { - value: { current: value }, - version: { current: version }, - listeners, - } = contextValue; - const selected = selector(value); - - const [state, setState] = React.useState([value, selected]); - const dispatch = ( - payload: - | undefined // undefined from render below - | readonly [ContextVersion, Value], // from provider effect - ) => { - setState(prevState => { - if (!payload) { - // early bail out when is dispatched during render - return [value, selected] as const; - } - - if (payload[0] <= version) { - if (Object.is(prevState[1], selected)) { - return prevState; // bail out - } - - return [value, selected] as const; - } - - try { - if (Object.is(prevState[0], payload[1])) { - return prevState; // do not update - } - - const nextSelected = selector(payload[1]); - - if (Object.is(prevState[1], nextSelected)) { - return prevState; // do not update - } + useIsomorphicLayoutEffect(() => { + selectorRef.current = selector; + lastReturnedRef.current = selectedAtRender; + }); - return [payload[1], nextSelected] as const; - } catch (e) { - // ignored (stale props or some other reason) + useIsomorphicLayoutEffect(() => { + const listener = (payload: readonly [ContextVersion, Value]) => { + const next = selectorRef.current(payload[1]); + if (!Object.is(lastReturnedRef.current, next)) { + forceUpdate(); } + }; + listeners.push(listener); - // explicitly spread to enforce typing - return [prevState[0], prevState[1]] as const; // schedule update - }); - }; - - if (!Object.is(state[1], selected)) { - // schedule re-render - // this is safe because it's self contained - dispatch(undefined); - } - - const stableDispatch = useEventCallback(dispatch); - - useIsomorphicLayoutEffect(() => { - listeners.push(stableDispatch); + // Effect-fixup: catch updates that occurred between render and effect run + // (Relay's useFragmentInternal pattern). + const nextAtEffect = selectorRef.current(valueRef.current); + if (!Object.is(lastReturnedRef.current, nextAtEffect)) { + forceUpdate(); + } return () => { - const index = listeners.indexOf(stableDispatch); - listeners.splice(index, 1); + const index = listeners.indexOf(listener); + if (index !== -1) { + listeners.splice(index, 1); + } }; - }, [stableDispatch, listeners]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [listeners, valueRef]); - return state[1] as SelectedValue; + return selectedAtRender; }; From 038b6fbed0a7a30ec77ba32ac49496c9668f120d Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Sat, 18 Apr 2026 11:23:17 +0200 Subject: [PATCH 2/5] fix(react-context-selector): remove unused eslint-disable for exhaustive-deps The react-compiler rule errors when react-hooks rules are disabled, and exhaustive-deps doesn't actually complain about the effect's deps (both listeners and valueRef are referentially stable context fields). --- .../react-context-selector/src/useContextSelector.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-components/react-context-selector/src/useContextSelector.ts b/packages/react-components/react-context-selector/src/useContextSelector.ts index ba6ee59af9f0c0..f9b568c123ef6b 100644 --- a/packages/react-components/react-context-selector/src/useContextSelector.ts +++ b/packages/react-components/react-context-selector/src/useContextSelector.ts @@ -67,7 +67,6 @@ export const useContextSelector = ( listeners.splice(index, 1); } }; - // eslint-disable-next-line react-hooks/exhaustive-deps }, [listeners, valueRef]); return selectedAtRender; From 7ef2b35b05041c1d5c658737ad0fc4d8b76ac53c Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Sat, 18 Apr 2026 17:18:37 +0200 Subject: [PATCH 3/5] fix(react-context-selector): restore selector error isolation + regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addressing code review feedback on #36002: 1. [BLOCKER] Restore try/catch around selector calls in both the listener and the effect-fixup. The previous implementation wrapped the selector in a try/catch with the comment 'ignored (stale props or some other reason)'. The new hook dropped that guard; since the provider iterates listeners with Array.prototype.forEach which aborts on throw, any consumer whose selector throws on an intermediate payload would silently starve all later subscribers of that update. Restoring the guard preserves the prior isolation contract. 2. Add an inline comment explaining why the new hook destructures only { value, listeners } from the context and ignores the version field. The version is still maintained by the provider and consulted by useHasParentContext as a parent-presence sentinel; it is no longer needed as a staleness guard because the provider fires listeners synchronously inside useIsomorphicLayoutEffect, and freshness is guaranteed by the effect-fixup. 3. Add regression tests: - 'memoized consumers re-render only when their selected slice changes': the RFC's Scenario 2 with React.memo'd items, asserting per-item onUpdate call counts across a cycling sequence. Catches the 'three items re-rendered instead of two' glitch if it regresses. - 'a single consumer throw does not starve later subscribers of context updates': exercises the listener error-isolation path restored above. StrictMode sanity-checked locally: layout-effect mount → cleanup → mount in dev produces idempotent ref re-assignments and no extra forceUpdate() calls — render counts scale by React's dev-mode 2x factor as expected, with no pathological churn. --- .../src/useContextSelector.test.tsx | 98 +++++++++++++++++++ .../src/useContextSelector.ts | 31 ++++-- 2 files changed, 123 insertions(+), 6 deletions(-) diff --git a/packages/react-components/react-context-selector/src/useContextSelector.test.tsx b/packages/react-components/react-context-selector/src/useContextSelector.test.tsx index 4b609f83500256..60eb54be4b9c16 100644 --- a/packages/react-components/react-context-selector/src/useContextSelector.test.tsx +++ b/packages/react-components/react-context-selector/src/useContextSelector.test.tsx @@ -98,4 +98,102 @@ describe('useContextSelector', () => { expect(document.querySelector('.test-component')?.dataset.active).toBe('true'); expect(onUpdate).toHaveBeenCalledTimes(2); }); + + // Regression test for the eager-bailout glitch fixed in this hook rewrite. + // With the previous `[value, selected]` + `setState(prev => prev)` bailout, + // React's eager path silently stopped working after the first listener-driven + // render committed (the bound-at-mount fiber becomes the stale alternate and + // its .lanes never clear). That resulted in extra component-function calls + // on subsequent context updates, even when the selected slice didn't change. + // + // This test runs the RFC's Scenario 2 (controlled List + memoized items) and + // asserts that each memoized item re-renders only when its `active` flag + // actually flipped — no "three items re-rendered instead of two" anomaly. + it('memoized consumers re-render only when their selected slice changes', () => { + const MemoizedTestComponent = React.memo(TestComponent); + + const onUpdates: Record = { + 1: jest.fn(), + 2: jest.fn(), + 3: jest.fn(), + 4: jest.fn(), + }; + + render( + + + + + + , + { container: container as HTMLElement }, + ); + + // Initial render. TestProvider starts at index=0, so none are active. + // Each memoized item has committed once — the `onUpdate` effect fires + // once per item on mount. + Object.values(onUpdates).forEach(m => expect(m).toHaveBeenCalledTimes(1)); + + // Click 1: 0 → 1. Only index=1 flips (false → true). + act(() => { + document.querySelector('.test-provider')?.click(); + }); + expect(onUpdates[1]).toHaveBeenCalledTimes(2); + expect(onUpdates[2]).toHaveBeenCalledTimes(1); + expect(onUpdates[3]).toHaveBeenCalledTimes(1); + expect(onUpdates[4]).toHaveBeenCalledTimes(1); + + // Click 2: 1 → 2. index=1 flips true→false, index=2 flips false→true. + // Items 3 and 4 did not change and must not re-render. + act(() => { + document.querySelector('.test-provider')?.click(); + }); + expect(onUpdates[1]).toHaveBeenCalledTimes(3); + expect(onUpdates[2]).toHaveBeenCalledTimes(2); + expect(onUpdates[3]).toHaveBeenCalledTimes(1); // ← was 2 under the old eager-bailout glitch + expect(onUpdates[4]).toHaveBeenCalledTimes(1); + + // Click 3: 2 → 3. + act(() => { + document.querySelector('.test-provider')?.click(); + }); + expect(onUpdates[1]).toHaveBeenCalledTimes(3); + expect(onUpdates[2]).toHaveBeenCalledTimes(3); + expect(onUpdates[3]).toHaveBeenCalledTimes(2); + expect(onUpdates[4]).toHaveBeenCalledTimes(1); + }); + + it('a single consumer throw does not starve later subscribers of context updates', () => { + const ThrowingConsumer: React.FC<{ threshold: number }> = ({ threshold }) => { + useContextSelector(TestContext, v => { + if (v.index > threshold) { + throw new Error('selector cannot handle this value'); + } + return v.index; + }); + return null; + }; + + const onUpdate = jest.fn(); + + render( + + + + , + { container: container as HTMLElement }, + ); + + expect(onUpdate).toHaveBeenCalledTimes(1); + + // Click: index goes 0 → 1. The first consumer's selector will throw + // (1 > 0). The downstream TestComponent must still receive the update + // because listener error isolation prevents `listeners.forEach` from + // short-circuiting. + act(() => { + document.querySelector('.test-provider')?.click(); + }); + expect(document.querySelector('.test-component')?.dataset.active).toBe('true'); + expect(onUpdate).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/react-components/react-context-selector/src/useContextSelector.ts b/packages/react-components/react-context-selector/src/useContextSelector.ts index f9b568c123ef6b..5b6b818e68dbf5 100644 --- a/packages/react-components/react-context-selector/src/useContextSelector.ts +++ b/packages/react-components/react-context-selector/src/useContextSelector.ts @@ -17,6 +17,13 @@ export const useContextSelector = ( selector: ContextSelector, ): SelectedValue => { const contextValue = React.useContext(context as unknown as Context>); + // `version` intentionally unused here. It is still maintained by the + // provider and consulted by `useHasParentContext` as a "has-provider" + // sentinel, but this hook no longer rejects listener payloads by version: + // the provider increments the version and fires listeners synchronously in + // a single `useIsomorphicLayoutEffect` tick, so a listener payload can't be + // older than the committed state by the time we evaluate it. Freshness is + // instead guaranteed by the effect-fixup below. const { value: valueRef, listeners } = contextValue; // Read valueRef during render and return selector(value) directly. This is @@ -47,18 +54,30 @@ export const useContextSelector = ( useIsomorphicLayoutEffect(() => { const listener = (payload: readonly [ContextVersion, Value]) => { - const next = selectorRef.current(payload[1]); - if (!Object.is(lastReturnedRef.current, next)) { - forceUpdate(); + // Selectors can throw on transiently-inconsistent inputs (stale props + // vs. newer context value is the classic case). Swallow so a single + // consumer's throw doesn't abort the provider's `listeners.forEach` + // and starve later subscribers of this update. + try { + const next = selectorRef.current(payload[1]); + if (!Object.is(lastReturnedRef.current, next)) { + forceUpdate(); + } + } catch { + // ignored (stale props or similar — heals on the next parent-driven render) } }; listeners.push(listener); // Effect-fixup: catch updates that occurred between render and effect run // (Relay's useFragmentInternal pattern). - const nextAtEffect = selectorRef.current(valueRef.current); - if (!Object.is(lastReturnedRef.current, nextAtEffect)) { - forceUpdate(); + try { + const nextAtEffect = selectorRef.current(valueRef.current); + if (!Object.is(lastReturnedRef.current, nextAtEffect)) { + forceUpdate(); + } + } catch { + // ignored — same rationale as the listener above } return () => { From 3cc3c8a3590f19def1af50b5524e36f5642178e2 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Mon, 20 Apr 2026 11:16:16 +0200 Subject: [PATCH 4/5] fixing slop --- .../etc/react-context-selector.api.md | 18 ++---- .../src/createContext.test.tsx | 5 +- .../src/createContext.ts | 8 +-- .../react-context-selector/src/index.ts | 2 +- .../react-context-selector/src/types.ts | 23 ++----- .../src/useContextSelector.test.tsx | 54 ++++++++-------- .../src/useContextSelector.ts | 63 ++++++------------- .../src/useHasParentContext.ts | 6 +- 8 files changed, 65 insertions(+), 114 deletions(-) diff --git a/packages/react-components/react-context-selector/etc/react-context-selector.api.md b/packages/react-components/react-context-selector/etc/react-context-selector.api.md index de10516c8addb9..8027f3ce9f5952 100644 --- a/packages/react-components/react-context-selector/etc/react-context-selector.api.md +++ b/packages/react-components/react-context-selector/etc/react-context-selector.api.md @@ -17,24 +17,18 @@ export type ContextSelector = (value: Value) => SelectedVa // @internal (undocumented) export type ContextValue = { - listeners: ((payload: readonly [ContextVersion, Value]) => void)[]; - value: React_2.MutableRefObject; - version: React_2.MutableRefObject; + listeners: ((payload: Value) => void)[]; + value: { + current: Value; + }; + isDefault?: boolean; }; -// @internal (undocumented) -export type ContextValues = ContextValue & { - listeners: ((payload: readonly [ContextVersion, Record]) => void)[]; -}; - -// @internal (undocumented) -export type ContextVersion = number; - // @internal (undocumented) export const createContext: (defaultValue: Value) => Context; // @internal -export const useContextSelector: (context: Context, selector: ContextSelector) => SelectedValue; +export const useContextSelector: (context: Context, selectorFn: ContextSelector) => SelectedValue; // @internal export function useHasParentContext(context: Context): boolean; diff --git a/packages/react-components/react-context-selector/src/createContext.test.tsx b/packages/react-components/react-context-selector/src/createContext.test.tsx index da630ff14a7dad..1275075b00cdc5 100644 --- a/packages/react-components/react-context-selector/src/createContext.test.tsx +++ b/packages/react-components/react-context-selector/src/createContext.test.tsx @@ -1,9 +1,10 @@ import { createContext } from './createContext'; -import * as ReactIs from 'react-is'; +import { isValidElementType } from 'react-is'; describe('createContext', () => { it('creates a Provider component', () => { const Context = createContext(null); - expect(ReactIs.isValidElementType(Context.Provider)).toBeTruthy(); + + expect(isValidElementType(Context.Provider)).toBeTruthy(); }); }); diff --git a/packages/react-components/react-context-selector/src/createContext.ts b/packages/react-components/react-context-selector/src/createContext.ts index b8d0b2d3bf2ba4..9650e3158fea51 100644 --- a/packages/react-components/react-context-selector/src/createContext.ts +++ b/packages/react-components/react-context-selector/src/createContext.ts @@ -10,8 +10,6 @@ const createProvider = (Original: React.Provider>) => const Provider: React.FC> = props => { // Holds an actual "props.value" const valueRef = React.useRef(props.value); - // Used to sync context updates and avoid stale values, can be considered as render/effect counter of Provider. - const versionRef = React.useRef(0); // A stable object, is used to avoid context updates via mutation of its values. const contextValue = React.useRef>(null); @@ -19,18 +17,16 @@ const createProvider = (Original: React.Provider>) => if (!contextValue.current) { contextValue.current = { value: valueRef, - version: versionRef, listeners: [], }; } useIsomorphicLayoutEffect(() => { valueRef.current = props.value; - versionRef.current += 1; runWithPriority(NormalPriority, () => { (contextValue.current as ContextValue).listeners.forEach(listener => { - listener([versionRef.current, props.value]); + listener(props.value); }); }); }, [props.value]); @@ -53,8 +49,8 @@ export const createContext = (defaultValue: Value): Context => { // eslint-disable-next-line @fluentui/no-context-default-value const context = React.createContext>({ value: { current: defaultValue }, - version: { current: -1 }, listeners: [], + isDefault: true, }); context.Provider = createProvider(context.Provider); diff --git a/packages/react-components/react-context-selector/src/index.ts b/packages/react-components/react-context-selector/src/index.ts index bbc42ef78afce3..700c10680a4bff 100644 --- a/packages/react-components/react-context-selector/src/index.ts +++ b/packages/react-components/react-context-selector/src/index.ts @@ -2,4 +2,4 @@ export { createContext } from './createContext'; export { useContextSelector } from './useContextSelector'; export { useHasParentContext } from './useHasParentContext'; // eslint-disable-next-line @fluentui/ban-context-export -export type { Context, ContextSelector, ContextValue, ContextValues, ContextVersion } from './types'; +export type { Context, ContextSelector, ContextValue } from './types'; diff --git a/packages/react-components/react-context-selector/src/types.ts b/packages/react-components/react-context-selector/src/types.ts index d01f4d65e897d3..e08e8f4aa35c01 100644 --- a/packages/react-components/react-context-selector/src/types.ts +++ b/packages/react-components/react-context-selector/src/types.ts @@ -10,31 +10,16 @@ export type Context = React.Context & { export type ContextSelector = (value: Value) => SelectedValue; -/** - * @internal - */ -export type ContextVersion = number; - /** * @internal */ export type ContextValue = { /** Holds a set of subscribers from components. */ - listeners: ((payload: readonly [ContextVersion, Value]) => void)[]; + listeners: ((payload: Value) => void)[]; /** Holds an actual value of React's context that will be propagated down for computations. */ - value: // eslint-disable-next-line @typescript-eslint/no-deprecated - React.MutableRefObject; + value: { current: Value }; - /** A version field is used to sync a context value and consumers. */ - version: // eslint-disable-next-line @typescript-eslint/no-deprecated - React.MutableRefObject; -}; - -/** - * @internal - */ -export type ContextValues = ContextValue & { - /** List of listners to publish changes */ - listeners: ((payload: readonly [ContextVersion, Record]) => void)[]; + /** Indicates if a context holds default value. */ + isDefault?: boolean; }; diff --git a/packages/react-components/react-context-selector/src/useContextSelector.test.tsx b/packages/react-components/react-context-selector/src/useContextSelector.test.tsx index 60eb54be4b9c16..e99e9ca053c3d0 100644 --- a/packages/react-components/react-context-selector/src/useContextSelector.test.tsx +++ b/packages/react-components/react-context-selector/src/useContextSelector.test.tsx @@ -99,16 +99,11 @@ describe('useContextSelector', () => { expect(onUpdate).toHaveBeenCalledTimes(2); }); - // Regression test for the eager-bailout glitch fixed in this hook rewrite. - // With the previous `[value, selected]` + `setState(prev => prev)` bailout, - // React's eager path silently stopped working after the first listener-driven - // render committed (the bound-at-mount fiber becomes the stale alternate and - // its .lanes never clear). That resulted in extra component-function calls - // on subsequent context updates, even when the selected slice didn't change. + // Regression test for the eager-bailout issue with `useState(). // - // This test runs the RFC's Scenario 2 (controlled List + memoized items) and - // asserts that each memoized item re-renders only when its `active` flag - // actually flipped — no "three items re-rendered instead of two" anomaly. + // With the previous `[value, selected]` + `setState(prev => prev)` bailout, React's eager path silently stopped + // working after the first listener-driven render committed. That resulted in extra component-function calls + // on subsequent context updates, even when the selected slice didn't change. it('memoized consumers re-render only when their selected slice changes', () => { const MemoizedTestComponent = React.memo(TestComponent); @@ -129,38 +124,43 @@ describe('useContextSelector', () => { { container: container as HTMLElement }, ); - // Initial render. TestProvider starts at index=0, so none are active. - // Each memoized item has committed once — the `onUpdate` effect fires - // once per item on mount. + // Initial render. TestProvider starts at index=0, so none are active. Each memoized item has committed once — the + // `onUpdate` effect fires once per item on mount. Object.values(onUpdates).forEach(m => expect(m).toHaveBeenCalledTimes(1)); - // Click 1: 0 → 1. Only index=1 flips (false → true). + // Click 1: 0 → 1. Only index=1 flips (active: false → true). + jest.clearAllMocks(); act(() => { document.querySelector('.test-provider')?.click(); }); - expect(onUpdates[1]).toHaveBeenCalledTimes(2); - expect(onUpdates[2]).toHaveBeenCalledTimes(1); - expect(onUpdates[3]).toHaveBeenCalledTimes(1); - expect(onUpdates[4]).toHaveBeenCalledTimes(1); - // Click 2: 1 → 2. index=1 flips true→false, index=2 flips false→true. + expect(onUpdates[1]).toHaveBeenCalledTimes(1); // true => false + expect(onUpdates[2]).toHaveBeenCalledTimes(0); + expect(onUpdates[3]).toHaveBeenCalledTimes(0); + expect(onUpdates[4]).toHaveBeenCalledTimes(0); + + // Click 2: 1 → 2. index=1 flips (active: true → false), index=2 flips (active: false → true). // Items 3 and 4 did not change and must not re-render. + jest.clearAllMocks(); act(() => { document.querySelector('.test-provider')?.click(); }); - expect(onUpdates[1]).toHaveBeenCalledTimes(3); - expect(onUpdates[2]).toHaveBeenCalledTimes(2); - expect(onUpdates[3]).toHaveBeenCalledTimes(1); // ← was 2 under the old eager-bailout glitch - expect(onUpdates[4]).toHaveBeenCalledTimes(1); + + expect(onUpdates[1]).toHaveBeenCalledTimes(1); // true => false + expect(onUpdates[2]).toHaveBeenCalledTimes(1); // false => true + expect(onUpdates[3]).toHaveBeenCalledTimes(0); // ← was 2 under the old eager-bailout with `useState()` + expect(onUpdates[4]).toHaveBeenCalledTimes(0); // Click 3: 2 → 3. + jest.clearAllMocks(); act(() => { document.querySelector('.test-provider')?.click(); }); - expect(onUpdates[1]).toHaveBeenCalledTimes(3); - expect(onUpdates[2]).toHaveBeenCalledTimes(3); - expect(onUpdates[3]).toHaveBeenCalledTimes(2); - expect(onUpdates[4]).toHaveBeenCalledTimes(1); + + expect(onUpdates[1]).toHaveBeenCalledTimes(0); + expect(onUpdates[2]).toHaveBeenCalledTimes(1); // true => false + expect(onUpdates[3]).toHaveBeenCalledTimes(1); // false => true + expect(onUpdates[4]).toHaveBeenCalledTimes(0); }); it('a single consumer throw does not starve later subscribers of context updates', () => { @@ -171,6 +171,7 @@ describe('useContextSelector', () => { } return v.index; }); + return null; }; @@ -193,6 +194,7 @@ describe('useContextSelector', () => { act(() => { document.querySelector('.test-provider')?.click(); }); + expect(document.querySelector('.test-component')?.dataset.active).toBe('true'); expect(onUpdate).toHaveBeenCalledTimes(2); }); diff --git a/packages/react-components/react-context-selector/src/useContextSelector.ts b/packages/react-components/react-context-selector/src/useContextSelector.ts index 5b6b818e68dbf5..8b5c12533ce2a4 100644 --- a/packages/react-components/react-context-selector/src/useContextSelector.ts +++ b/packages/react-components/react-context-selector/src/useContextSelector.ts @@ -3,7 +3,7 @@ import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; import * as React from 'react'; -import type { Context, ContextSelector, ContextValue, ContextVersion } from './types'; +import type { Context, ContextSelector, ContextValue } from './types'; /** * This hook returns context selected value by selector. @@ -14,79 +14,56 @@ import type { Context, ContextSelector, ContextValue, ContextVersion } from './t */ export const useContextSelector = ( context: Context, - selector: ContextSelector, + selectorFn: ContextSelector, ): SelectedValue => { const contextValue = React.useContext(context as unknown as Context>); - // `version` intentionally unused here. It is still maintained by the - // provider and consulted by `useHasParentContext` as a "has-provider" - // sentinel, but this hook no longer rejects listener payloads by version: - // the provider increments the version and fires listeners synchronously in - // a single `useIsomorphicLayoutEffect` tick, so a listener payload can't be - // older than the committed state by the time we evaluate it. Freshness is - // instead guaranteed by the effect-fixup below. const { value: valueRef, listeners } = contextValue; - // Read valueRef during render and return selector(value) directly. This is - // analogous to useSyncExternalStore's getSnapshot and is the only way to - // select a slice from a shared ref-based store without re-rendering every + // Read valueRef during render and return selector(value) directly. This is analogous to `useSyncExternalStore`'s + // `getSnapshot` and is the only way to select a slice from a shared ref-based store without re-rendering every // consumer on every provider update. - const selectedAtRender = selector(valueRef.current); - - // Opaque force-update. Kept deliberately separate from a `[value, selected]` - // useState tuple: the previous implementation relied on React's eager-bailout - // fast path (`setState(prev => prev)` → `Object.is(eagerState, currentState)`) - // which silently stops firing after any listener-driven render commits — the - // fiber bound at mount becomes the stale alternate and its `fiber.lanes` - // retains SyncLane, permanently failing the `NoLanes` precondition of - // `dispatchSetStateInternal`. See RFC addendum "Option D" for details. + const valueAtRender = selectorFn(valueRef.current); const [, forceUpdate] = React.useReducer((x: number) => x + 1, 0); // Refs holding the current selector and the most-recently-returned slice. // Updated in a layout effect (ordering: children first, then provider) so // they are current by the time the provider's listener loop fires. - const selectorRef = React.useRef>(selector); - const lastReturnedRef = React.useRef(selectedAtRender); + const selectorFnRef = React.useRef>(selectorFn); + const lastValueAtRender = React.useRef(valueAtRender); useIsomorphicLayoutEffect(() => { - selectorRef.current = selector; - lastReturnedRef.current = selectedAtRender; + selectorFnRef.current = selectorFn; + lastValueAtRender.current = valueAtRender; }); useIsomorphicLayoutEffect(() => { - const listener = (payload: readonly [ContextVersion, Value]) => { - // Selectors can throw on transiently-inconsistent inputs (stale props - // vs. newer context value is the classic case). Swallow so a single - // consumer's throw doesn't abort the provider's `listeners.forEach` - // and starve later subscribers of this update. + const listener = (payload: Value) => { + // Selectors can throw on transiently-inconsistent inputs (stale props vs. newer context value). Swallow so a + // single consumer's throw doesn't abort the provider's `listeners.forEach`. try { - const next = selectorRef.current(payload[1]); - if (!Object.is(lastReturnedRef.current, next)) { + const nextSelectedValue = selectorFnRef.current(payload); + + if (!Object.is(lastValueAtRender.current, nextSelectedValue)) { forceUpdate(); } } catch { // ignored (stale props or similar — heals on the next parent-driven render) } }; + listeners.push(listener); - // Effect-fixup: catch updates that occurred between render and effect run - // (Relay's useFragmentInternal pattern). - try { - const nextAtEffect = selectorRef.current(valueRef.current); - if (!Object.is(lastReturnedRef.current, nextAtEffect)) { - forceUpdate(); - } - } catch { - // ignored — same rationale as the listener above - } + // Effect-fixup: catch updates that occurred between render and effect run (Relay's useFragmentInternal pattern). + listener(valueRef.current); return () => { const index = listeners.indexOf(listener); + if (index !== -1) { listeners.splice(index, 1); } }; }, [listeners, valueRef]); - return selectedAtRender; + return valueAtRender; }; diff --git a/packages/react-components/react-context-selector/src/useHasParentContext.ts b/packages/react-components/react-context-selector/src/useHasParentContext.ts index bc78fbfb4f8a71..4754c058b05bc1 100644 --- a/packages/react-components/react-context-selector/src/useHasParentContext.ts +++ b/packages/react-components/react-context-selector/src/useHasParentContext.ts @@ -14,9 +14,5 @@ import type { Context, ContextValue } from './types'; export function useHasParentContext(context: Context): boolean { const contextValue = React.useContext(context as unknown as Context>); - if (contextValue.version) { - return contextValue.version.current !== -1; - } - - return false; + return !contextValue.isDefault; } From 48281a02a3a9c75023317ba70d96bf3fe0c8f70c Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Thu, 23 Apr 2026 12:25:58 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Oleksandr Fediashov --- .../react-context-selector/src/useContextSelector.test.tsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/react-components/react-context-selector/src/useContextSelector.test.tsx b/packages/react-components/react-context-selector/src/useContextSelector.test.tsx index e99e9ca053c3d0..3a0d9f5d8b1dab 100644 --- a/packages/react-components/react-context-selector/src/useContextSelector.test.tsx +++ b/packages/react-components/react-context-selector/src/useContextSelector.test.tsx @@ -99,11 +99,6 @@ describe('useContextSelector', () => { expect(onUpdate).toHaveBeenCalledTimes(2); }); - // Regression test for the eager-bailout issue with `useState(). - // - // With the previous `[value, selected]` + `setState(prev => prev)` bailout, React's eager path silently stopped - // working after the first listener-driven render committed. That resulted in extra component-function calls - // on subsequent context updates, even when the selected slice didn't change. it('memoized consumers re-render only when their selected slice changes', () => { const MemoizedTestComponent = React.memo(TestComponent);