refactor(react-virtual): subscribe to virtualizer updates with useSyncExternalStore - #1259
refactor(react-virtual): subscribe to virtualizer updates with useSyncExternalStore#1259kklem0 wants to merge 1 commit into
Conversation
…cExternalStore Replace the reducer bump in useVirtualizerBase with a small external store consumed through useSyncExternalStore (official use-sync-external-store shim, so the >=16.8 peer range is unchanged). useVirtualizer / useWindowVirtualizer keep the same API and identity semantics; consumers still read getVirtualItems() / getTotalSize() from the instance. The store snapshot is a version counter bumped by every notification the adapter decides to render, so under concurrent rendering React can detect a mid-render store change and re-render synchronously instead of committing a torn range. useFlushSync keeps its meaning: the sync scroll path notifies inside flushSync. useSyncExternalStore subscribes in a passive effect, so notifications raised while React is committing (initial rect/offset measurement in _willUpdate, scroll-element swaps, measureElement refs) would only surface after paint. A final layout effect compares the store version with the rendered one and dispatches a reducer so those still re-render before paint, exactly as the reducer-only implementation did; mount/measure render counts are unchanged. Tests cover scroll notifications through the store, useFlushSync sync vs scheduled commits outside act, SSR via getServerSnapshot, StrictMode, and post-unmount notifications. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
ChangesReact virtualizer store
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This refactors virtualizer update subscriptions without changing the public API or instance identity semantics; the reported validation passes, and no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant useVirtualizerBase
participant store
participant React
useVirtualizerBase->>store: notify version onChange
store->>React: notify subscribed snapshot
React->>useVirtualizerBase: render current version
useVirtualizerBase->>React: reconcile version in layout effect
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
packages/react-virtual/tests/index.test.tsxParsing error: "parserOptions.project" has been provided for Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
1 participant
🎯 Changes
Drives
useVirtualizer/useWindowVirtualizerre-renders throughuseSyncExternalStoreinstead of auseReducerbump. The public API, the returned instance and its identity semantics are unchanged — consumers keep callinggetVirtualItems()/getTotalSize()on the instance.This is the direction suggested in #851 ("
useSyncExternalStorefeels like a more natural fit compared touseMemo"), applied to the existing hooks rather than as a separate hook: no new hook names, no API change, and nothing that tries to work around React Compiler'sknownIncompatibleentry foruseVirtualizer.It is the minimal alternative to #1241 (which adds separate
useVirtualizerSnapshothooks). Both are open on purpose so you can pick the shape you prefer — they don't have to be exclusive, see the React Compiler note below.Design
directDomUpdatesgating is untouched). The counter tells React that the instance moved; consumers still read render-facing values from the instance.useFlushSyncpreserved: the synchronous scroll path notifies insideflushSync, so the DOM still updates before the scroll handler returns.useSyncExternalStoresubscribes in a passive effect, so notifications raised while React is committing — the initial rect/offset measurement in_willUpdate, scroll-element swaps,measureElementrefs for freshly mounted items — have no listener yet and would only surface via the store's post-commit check, i.e. after paint (a visible flash of the unmeasured range on heavy initial renders). A final layout effect compares the store version with the rendered one and dispatches a reducer so those still re-render synchronously before paint, exactly as before. Mount / measure render counts in the existing tests are unchanged (2 and 3).use-sync-external-store/shim(React's official package) keeps the^16.8 || ^17 || ^18 || ^19peer range; on React 18+ it isReact.useSyncExternalStore.getServerSnapshotprovided;renderToStringcovered by a test.Why
useDeferredValue) a reducer bump scheduled while a render is in progress lets React finish and commit that render with the stale range it already read, then re-render. WithuseSyncExternalStoreReact checks the store at the end of a non-blocking render and re-renders synchronously if it moved, so a torn frame is never committed.useSyncExternalStoreis the primitive React provides for that.Tests
isScrollingflip),useFlushSyncsync commit vs. React-scheduled commit outsideact, SSR viagetServerSnapshot, StrictMode, no updates after unmount.pnpm run test:prlocally: eslint, types, lib, build/publint, sherif, knip, and the full react-virtual Playwright suite (33 tests, incl.react-compileranddirect-dom-updates).Note on React Compiler /
knownIncompatibleReact Compiler ships a hardcoded
knownIncompatibleentry for@tanstack/react-virtual'suseVirtualizerand skips every component that calls it (#1119). This PR deliberately does not change that, and doesn't try to work around it:getVirtualItems()on it (React Compiler: useVirtual results are cached at their initial values #736, React 19 issue with useWindowVirtualizer #743). The entry stays accurate for now; this PR only changes how the adapter subscribes.Possible follow-ups, as separate PRs, whichever you prefer:
virtualItems,totalSize) next to the stable instance for imperative calls. feat(react-virtual): add useVirtualizerSnapshot for React Compiler compatibility via useSyncExternalStore #1241 is one concrete shape of that; it can be rebased onto this store.knownIncompatibleentry, so consumers compile without renaming imports.Refs: #851, #736, #743, #1119, #1241
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit