Skip to content

Commit 9a5e837

Browse files
authored
RFC: useContextSelector() amend with Option D (Relay pattern) (#36001)
1 parent 7dfa6a3 commit 9a5e837

1 file changed

Lines changed: 115 additions & 1 deletion

File tree

docs/react-v9/contributing/rfcs/react-components/context-selector-tearing.md

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,25 @@
99
- [Current problem](#current-problem)
1010
- [What is _tearing_?](#what-is-_tearing_)
1111
- [Where we are _now_?](#where-we-are-_now_)
12+
- [Behavioral benchmark](#behavioral-benchmark)
13+
- [`useContextSelector()` & re-renders](#usecontextselector--re-renders)
14+
- [Test results](#test-results)
15+
- [`@fluentui/react-context-selector` & re-renders (we use this)](#fluentuireact-context-selector--re-renders-we-use-this)
16+
- [Test results](#test-results-1)
1217
- [Exploration](#exploration)
18+
- [Tearing issues](#tearing-issues)
1319
- [POC](#poc)
14-
- [Options](#options)
20+
- [Can we avoid `useLayoutEffect`?](#can-we-avoid-uselayouteffect)
21+
- [Decision](#decision)
1522
- [Option A: Do nothing (safe)](#option-a-do-nothing-safe)
23+
- [Rejected options](#rejected-options)
1624
- [Option B: Use `useSyncExternalStore()` (probably safe)](#option-b-use-usesyncexternalstore-probably-safe)
1725
- [Option C: Propagate the value in render (risky)](#option-c-propagate-the-value-in-render-risky)
26+
- [Addendum (2026-04): Option D — Allow tearing, heal in effect](#addendum-2026-04-option-d--allow-tearing-heal-in-effect)
27+
- [The underlying bug aka "glitchy behavior"](#the-underlying-bug-aka-glitchy-behavior)
28+
- [Option D: Relay's `useFragmentInternal` pattern](#option-d-relays-usefragmentinternal-pattern)
29+
- [What changes vs. Option A](#what-changes-vs-option-a)
30+
- [New recommendation](#new-recommendation)
1831

1932
<!-- END doctoc generated TOC please keep comment here to allow auto update -->
2033

@@ -513,3 +526,104 @@ function Provider() {
513526
```
514527

515528
It will function as expected in terms of behavior, but _might_ introduce additional issues with tearing.
529+
530+
## Addendum (2026-04): Option D — Allow tearing, heal in effect
531+
532+
The original decision "Option A: do nothing" was made when the only known alternatives were `useSyncExternalStore` (Option B) and render-phase value propagation (Option C). An offline conversation with the React team after that decision surfaced a fourth option that the original RFC did not consider.
533+
534+
### The underlying bug aka "glitchy behavior"
535+
536+
The "glitchy behavior" note in the original RFC "three items re-rendered instead of two" has a precise root cause. `dispatchSetStateInternal` in `ReactFiberHooks.js` gates the eager-bailout fast path on:
537+
538+
```ts
539+
if (fiber.lanes === NoLanes && (alternate === null || alternate.lanes === NoLanes)) {
540+
// eager bailout — drop the update without scheduling a render
541+
}
542+
```
543+
544+
> https://github.com/facebook/react/blob/1ddff43c41147b880c22eb363e07aade5a71c5d9/packages/react-reconciler/src/ReactFiberHooks.js#L3648-L3651
545+
546+
The `fiber` passed is the fiber bound at mount via `queue.dispatch = dispatchSetState.bind(null, currentlyRenderingFiber, queue)`. After the first listener-driven render commits:
547+
548+
1. React clones `fiberA` (mount fiber) into WIP `fiberB`; `beginWork` clears `fiberB.lanes = NoLanes`.
549+
2. Commit swaps: `fiberB` becomes current, `fiberA` becomes alternate.
550+
3. `fiberA.lanes` was never cleared — it still holds `SyncLane` from the `enqueueUpdate` that preceded the render.
551+
552+
From that point on, every subsequent `dispatchSetState(fiberA, ...)` fails the `NoLanes` precondition. The eager path is **permanently unavailable** for that component instance. Updates are enqueued 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 ran (_and causes perf issues_), which is exactly the pattern the original "glitchy behavior" log showed.
553+
554+
This is confirmed by patching `react-dom.development.js` to log `fiber.lanes` / `alternate.lanes` at the eager-bailout check in `dispatchSetState`.
555+
556+
> 💡A standalone reproduction running the RFC's Scenario 2 against the published `@fluentui/react-context-selector@9.2.15` is at [layershifter/context-selector-bailout-repro](https://github.com/layershifter/context-selector-bailout-repro).
557+
>
558+
> Sample output from that probe. Four memoized `ListItem` components, `activeValue` cycled through the `set-N` buttons. For each click the probe shows which components re-rendered and the eager-bailout-gate observation on each `dispatchSetState` fired during that click. `SKIPPED ✗` means the precondition `fiber.lanes === NoLanes && (alternate === null || alternate.lanes === NoLanes)` failed and the update fell through to the non-eager path:
559+
>
560+
> <details>
561+
> <summary>Log output</summary>
562+
> ```
563+
> == click 1 (1→2) → activeValue=2 ==
564+
> Component function invocations (expected 2: items 1 and 2):
565+
> ListItem[1] isActive=false
566+
> ListItem[2] isActive=true
567+
> dispatchSetState → eager-bailout check:
568+
> App fiber=0 alt=null EAGER-PATH ✓
569+
> ListItem fiber=0 alt=null EAGER-PATH ✓ ← first dispatch on a given fiber, clean
570+
> ListItem fiber=1 alt=null SKIPPED ✗ ← sibling hook on the SAME fiber, now polluted
571+
> ListItem fiber=0 alt=null EAGER-PATH ✓
572+
> ListItem fiber=1 alt=null SKIPPED ✗
573+
> ListItem fiber=0 alt=null EAGER-PATH ✓
574+
> ListItem fiber=0 alt=null EAGER-PATH ✓
575+
> ListItem fiber=0 alt=null EAGER-PATH ✓
576+
> ListItem fiber=0 alt=null EAGER-PATH ✓
577+
>
578+
> == click 2 (2→3) → activeValue=3 ← "three items rendered instead of two" ==
579+
> Component function invocations (expected 2: items 2 and 3; actual 3):
580+
> ListItem[1] isActive=false ← should NOT have re-rendered
581+
> ListItem[2] isActive=false
582+
> ListItem[3] isActive=true
583+
> dispatchSetState → eager-bailout check:
584+
> App fiber=0 alt=0 EAGER-PATH ✓
585+
> ListItem fiber=1 alt=0 SKIPPED ✗ ← items 1, 2 previously committed,
586+
> ListItem fiber=1 alt=1 SKIPPED ✗ fiber.lanes retained SyncLane from
587+
> ListItem fiber=1 alt=0 SKIPPED ✗ the prior enqueueUpdate — every
588+
> ListItem fiber=1 alt=1 SKIPPED ✗ subsequent dispatch fails the
589+
> ListItem fiber=0 alt=0 EAGER-PATH ✓ NoLanes precondition
590+
> ListItem fiber=1 alt=1 SKIPPED ✗
591+
> ListItem fiber=0 alt=0 EAGER-PATH ✓ ← items 3, 4 never committed a
592+
> ListItem fiber=0 alt=0 EAGER-PATH ✓ listener-driven render yet, still clean
593+
>
594+
> ```
595+
> </details>
596+
> ```
597+
598+
- In `click 1`, items 3 and 4 had never received a listener-driven render; their bound fibers still satisfy `fiber.lanes === NoLanes`. Items 1 and 2 fire two dispatches each (two `useContextSelector` hooks per `ListItem`); within the listener `forEach`, the second hook's dispatch sees `fiber.lanes === SyncLane` (set by the first hook's `enqueueUpdate`) and is forced off the eager path. This is the "within-batch" pollution.
599+
- In `click 2`, items 1 and 2 have committed once already, so their bound mount-fibers are now the stale alternates with `fiber.lanes === SyncLane` still set. Every dispatch on them is `SKIPPED ✗` from the first attempt. The reducer runs during the subsequent render, returns `prevState`, `didReceiveUpdate === false`, and React calls `bailoutOnAlreadyFinishedWork` — discarding the JSX but having already executed the component function. Item 1's render thus shows up in the `renderLog` even though its `isActive` never changed.
600+
601+
### Option D: Relay's `useFragmentInternal` pattern
602+
603+
[josephsavona](https://github.com/josephsavona) pointed out Relay's solution for an analogous problem (_consumers of a shared mutable store, wanting selective re-rendering_) and landed on:
604+
605+
- Provider updates value in `useLayoutEffect` (no render-phase writes)
606+
- Hook reads `valueRef.current` during render and returns `selector(value)` directly — analogous to `useSyncExternalStore`'s `getSnapshot`.
607+
- Hook holds only an opaque force-update counter (`useReducer(x => x + 1)`), not a `[value, selected]` tuple. **No `setState(prev => prev)` bailout trick is used**, so `fiber.lanes` is never polluted with would-have-bailed updates.
608+
- Listener compares `selector(newValue)` against the most-recently-returned slice (tracked in a ref updated in a layout effect). Calls `forceUpdate()` only when the selected slice actually changed.
609+
- Layout-effect fixup catches updates that occurred between render and effect firing.
610+
611+
> Relay's canonical implementation (permalinked to Relay commit `e3c9d1b`): the subscription layout effect [`useFragmentInternal_CURRENT.js#L600-L624`](https://github.com/facebook/relay/blob/e3c9d1b5661fb6f58e4d58f81cd930e09a47a89a/packages/react-relay/relay-hooks/useFragmentInternal_CURRENT.js#L600-L624) and the [`handleMissedUpdates` function at L136-207](https://github.com/facebook/relay/blob/e3c9d1b5661fb6f58e4d58f81cd930e09a47a89a/packages/react-relay/relay-hooks/useFragmentInternal_CURRENT.js#L136-L207).
612+
613+
### What changes vs. Option A
614+
615+
| | Option A (current main) | Option D |
616+
| ---------------------------------: | :----------------------------------------------- | :------------------------------------------------------------------- |
617+
| "Glitchy Behavior" (memo, +1 item) | Present | Fixed ✅ |
618+
| "Scenario 1 update" plain items | N items re-render with stale + 2 more afterwards | N items re-render, healed by listener ✅ |
619+
| Parent-driven tear | Level 1 (acceptable) | Level 1 (unchanged — Option D does not change this) |
620+
| Concurrent-mode correctness | Safe | Safe |
621+
| `react-hooks/refs` lint | Clean | Clean (one `getSnapshot`-style read, same as `useSyncExternalStore)` |
622+
623+
Option D is strictly better than Option A on the "Glitchy Behavior" axis and strictly equivalent everywhere else. It does NOT address Level 1 tearing — that remains out of scope per the original decision.
624+
625+
### New recommendation
626+
627+
Adopt Option D as the replacement implementation of `useContextSelector`. It is a drop-in replacement (same signature, same `ContextValue` shape).
628+
629+
Keep Option A as the "what we shipped before" record for context.

0 commit comments

Comments
 (0)