fix: react adapter sync state - #6458
Conversation
|
View your CI Pipeline Execution ↗ for commit 46921d5
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughControlled table state synchronization now aligns with React commit boundaries. React-specific selectors and reactivity bindings were added, core atom synchronization was deferred and comparator-aware, and pagination tests and examples verify controlled and external-atom updates. ChangesReact commit-aligned table reactivity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant React
participant useTable
participant TableStore
participant BaseAtoms
React->>useTable: render controlled state
useTable->>TableStore: select current table state
React->>useTable: run committed layout effect
useTable->>BaseAtoms: synchronize controlled state
useTable->>TableStore: commit reactive publication
TableStore-->>React: notify selected subscribers
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
| * Isolated `table.Subscribe` consumers still receive the underlying store | ||
| * notification; only this hook's root subscription is filtered. | ||
| */ | ||
| export function useTableSelector<TState, TSelected>( |
There was a problem hiding this comment.
fyi this is just a useSyncWithExternalStoreWithSelector revisited implementation
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/react-table/tests/useTable.test.tsx (1)
1-930: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared DOM-query helper.
The
container?.querySelector('[data-testid="..."]')?.textContentpattern is repeated across nearly every test. A small helper (e.g.queryText(testId: string)) would reduce boilerplate and keep future tests consistent.♻️ Example helper
function queryText(testId: string) { return container?.querySelector(`[data-testid="${testId}"]`)?.textContent }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-table/tests/useTable.test.tsx` around lines 1 - 930, Extract a shared queryText(testId) helper near render and reuse it throughout the tests in place of repeated container?.querySelector('[data-testid="..."]')?.textContent expressions. Update all data-testid lookups to call the helper while preserving their existing assertions and behavior.packages/react-table/src/reactivity.ts (1)
100-149: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
compareis frozen at first render whileselectoris kept live via ref.
selectorRefis updated every render so the latestselectoris always used, butcompareis captured directly by theuseStateinitializer closure and never refreshed — if a caller ever passes a non-stablecomparefunction, this hook would silently keep using the very first one forever. Today this is harmless because both call sites (useTable.ts) pass the same stableshallowreference, but this is an exported hook, so the inconsistency is a latent footgun for other/future callers.♻️ Proposed fix: mirror the selector-ref pattern for compare
const selectorRef = useRef(selector) selectorRef.current = selector + const compareRef = useRef(compare) + compareRef.current = compare const [selectedSource] = useState(() => { let hasSnapshot = false let snapshot: TSelected const getSnapshot = () => { const select = selectorRef.current ?? ((state: TState) => state as unknown as TSelected) const nextSnapshot = select(source.get()) - if (!hasSnapshot || !compare(snapshot, nextSnapshot)) { + if (!hasSnapshot || !compareRef.current(snapshot, nextSnapshot)) { snapshot = nextSnapshot hasSnapshot = true } return snapshot }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-table/src/reactivity.ts` around lines 100 - 149, Keep the comparison function current in useTableSelector by mirroring the existing selectorRef pattern: store compare in a ref and update it on every render, then have getSnapshot use the ref’s current comparator instead of the initializer-captured compare parameter. Preserve the existing snapshot caching and subscription behavior.packages/table-core/src/core/table/coreTablesFeature.utils.ts (1)
53-66: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueSkip
options.statesync for keys owned by external atoms.When a slice has both
options.atoms[key]andoptions.state[key],table.atoms[key]reads the external atom only, sotable_syncExternalStateToBaseAtoms()leavingbaseAtoms[key]stale is safe unless a setter readstable.baseAtoms[key]as the current value. Current feature code reads throughtable.atoms, so this is only a minor correctness cleanup.🛡️ Proposed fix
+import { hasOwn } from '../../utils' + table._reactivity.batch(() => { for (const key in state) { const baseAtom = (table.baseAtoms as Record<string, any>)[key] if (!baseAtom) { continue } + const externalAtoms = table.options.atoms as + | Record<string, unknown> + | undefined + if (externalAtoms && hasOwn(externalAtoms, key)) { + // An external atom owns this slice; `table.atoms[key]` never reads + // the base atom for it, so writing here would only leave stale data behind. + continue + } const externalState = state[key as keyof typeof state] const currentState = table._reactivity.untrack(() => baseAtom.get()) if (!compare(currentState, externalState)) { baseAtom.set(() => externalState) } } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/src/core/table/coreTablesFeature.utils.ts` around lines 53 - 66, Update table_syncExternalStateToBaseAtoms so it skips state synchronization for keys present in options.atoms, leaving those externally owned atoms untouched. Keep the existing baseAtoms lookup, comparison, and update behavior for keys without an external atom.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/react-table/src/reactivity.ts`:
- Around line 100-149: Keep the comparison function current in useTableSelector
by mirroring the existing selectorRef pattern: store compare in a ref and update
it on every render, then have getSnapshot use the ref’s current comparator
instead of the initializer-captured compare parameter. Preserve the existing
snapshot caching and subscription behavior.
In `@packages/react-table/tests/useTable.test.tsx`:
- Around line 1-930: Extract a shared queryText(testId) helper near render and
reuse it throughout the tests in place of repeated
container?.querySelector('[data-testid="..."]')?.textContent expressions. Update
all data-testid lookups to call the helper while preserving their existing
assertions and behavior.
In `@packages/table-core/src/core/table/coreTablesFeature.utils.ts`:
- Around line 53-66: Update table_syncExternalStateToBaseAtoms so it skips state
synchronization for keys present in options.atoms, leaving those externally
owned atoms untouched. Keep the existing baseAtoms lookup, comparison, and
update behavior for keys without an external atom.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58ab51b0-2435-4a26-bf73-f174554d6cb5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
examples/react/basic-external-atoms/src/main.tsxexamples/react/basic-external-atoms/tests/e2e/smoke.spec.tsexamples/react/basic-external-state/tests/e2e/smoke.spec.tspackages/react-table/package.jsonpackages/react-table/src/reactivity.tspackages/react-table/src/useTable.tspackages/react-table/tests/test-setup.tspackages/react-table/tests/useTable.test.tsxpackages/react-table/vite.config.tspackages/table-core/src/core/table/constructTable.tspackages/table-core/src/core/table/coreTablesFeature.utils.tspackages/table-core/tests/unit/core/tableAtoms.test.ts
Summary
This PR fixes React render-phase updates when using controlled table state, as reported in #6450.
The core idea is to separate two operations that previously happened together:
Previously,
useTablecalledsetOptions()during render, andsetOptions()immediately synchronized controlled state into writable base atoms. Updating those atoms synchronously notifiedtable.store, which could call React’s external-store listener while another render was still in progress:The new flow keeps render reads synchronous and current, but defers reactive publication until the React layout/commit phase.
Goals
The implementation is intended to support all of the following cases:
options.state.options.atoms.useSelector.table.Subscribeconsumers.getSnapshot()references.State ownership model
Each state slice now resolves using the documented precedence:
The check against
options.stateis an own-property check rather than a truthiness check. This distinguishes a missing controlled slice from a slice explicitly owned by the controlled state object.flowchart TD A[Read table state slice] --> B{External atom configured?} B -- Yes --> C[Read and return external atom] B -- No --> D[Read base atom to retain reactive dependency] D --> E{options.state owns this key?} E -- Yes --> F[Return controlled render value] E -- No --> G[Return base atom value] C --> H[Stable live slice snapshot] F --> H G --> H H --> I[Shallow-stable aggregate table.store] I --> J[React selector gate]The base or external atom is still read before resolving controlled state. This keeps the readonly atom connected to its reactive owner even while the controlled value currently wins.
Implementation
1. Live readonly atoms in the React adapter
React table options are plain values updated during render. They are not themselves reactive dependencies, so a normal computed atom cannot know that
options.statechanged.The React
createReadonlyAtomimplementation therefore returns a small live facade:get()evaluates the resolver against the latest render-time options.Conceptually:
This gives us two complementary behaviors:
2. Render phase: update options without publishing atoms
useTablenow updates the table options using:This keeps all options current during render but explicitly prevents
setOptions()from synchronizing controlled state into base atoms at that point.As a result, during the render that receives a new controlled value:
table.options.statecontains the new value.table.atoms.<slice>.get()returns the new value.table.store.get()returns the new value.table.statereturns the selected new value.3. Commit phase: publish the captured state
After React commits, an isomorphic layout effect publishes the controlled state captured by that render:
Using the captured state is important. The synchronization must represent the render that actually committed, rather than rereading a potentially newer mutable
table.optionsobject.The operation is batched so that:
The explicit commit invalidation also handles option-only changes where no base atom write occurs, such as releasing ownership of a controlled slice.
4. Root selector filtering
The public
table.storemust continue notifying its subscribers after a commit. This is required for consumers such as:However, the root
useTablecomponent has already rendered the new controlled value. Forwarding the same publication to React’s root external-store listener would be redundant.useTableSelectorwraps the existing TanStack StoreuseSelector; it does not reimplementuseSyncExternalStoreWithSelector.It records the selector and selected snapshot that React actually committed. When the public store later publishes:
Therefore:
table.Subscribecomponents can update.Hook ordering is intentional:
useTableSelectorrecords the committed root selection in its layout effect.useTablepublishes the atom graph in its following layout effect.5. Stable aggregate snapshots
The aggregate
table.storereadonly atom uses shallow comparison.This is necessary because building the full table state creates a new object. Without snapshot caching, React could observe a different object reference on every
getSnapshot()call and report:The store snapshot changes only when at least one resolved slice changes shallowly.
Controlled update sequence
The following sequence uses:
C0: previously committed controlled state.C1: next controlled state received by React.B0: previous base atom value.sequenceDiagram autonumber participant E as Table event participant R as React owner participant U as useTable participant O as table.options participant S as live table.store participant X as root useTableSelector participant B as base atoms participant C as commit atom participant I as isolated Subscribe E->>R: onPaginationChange(updater) R->>U: render with controlled C1 U->>O: table_setOptions(syncExternalState: false) Note over U,O: Options change during render<br/>No writable atom update U->>S: get current snapshot S->>B: read B0 to retain dependency Note over S: Resolve external atom<br/>then controlled C1<br/>then base B0 S-->>X: stable snapshot containing C1 X-->>U: selected C1 U-->>R: table.state, atoms and row models use C1 R->>R: commit UI for C1 Note over X,U: Layout phase after commit X->>X: record committed selection C1 U->>B: mirror captured C1, shallow-gated U->>C: increment commit version Note over U,C: Batched publication B-->>S: invalidate reactive dependencies C-->>S: invalidate option-dependent readonly atoms S->>I: publish committed snapshot C1 S->>X: publish committed snapshot C1 X->>X: compare next C1 with committed C1 Note over X: Equal selection<br/>skip root onStoreChange I->>I: rerender only if its selector changedSupported scenarios
baseAtoms[key]options.state[key]options.atoms[key]useSelectoris required.onStoreChangewhen its selected result is unchanged.API changes
table_setOptionsThe function accepts an optional final configuration argument:
The default remains unchanged:
continues to synchronize external state immediately. This preserves existing behavior for core and other adapters.
table_syncExternalStateToBaseAtomsThe function can now receive:
Passing an explicit snapshot allows React to publish the state associated with a specific committed render.
The implementation distinguishes:
from:
The first form reads
table.options.state. The second intentionally publishes no controlled state. This prevents an explicitly capturedundefinedvalue from accidentally falling back to a newer mutable options object.Usage examples
Controlled React state
The first and subsequent pagination updates now:
External atom without an outer subscription
No additional subscription is required:
The hidden readonly atom tracks the external atom, and
useTablereacts throughtable.store.Why not
queueMicrotask,setTimeout, or debouncing?A timer-based solution is not coupled to the React lifecycle.
Scheduling synchronization during render could cause a callback to run even if that render:
Timers also introduce ordering problems for rapid updates and can leave isolated subscribers stale for an arbitrary amount of time.
A passive effect would be tied to a real commit, but publication could happen after paint. A layout effect provides the required semantics:
Test coverage
The focused regression coverage includes:
useSelector, including row-model changestable.state, slice atom, flat store, subscriber and row model remain alignedstore.get(), fresh selector objects and recreated controlled slicesVerified focused results:
Additional validation performed:
Known limitation / non-goal
This change makes publication into the reactive atom graph commit-only, but it does not make the entire table instance render-local.
table.optionsis still a shared mutable object updated during render. Therefore, while a concurrent render is suspended or later abandoned, imperative reads such as:may observe render-in-progress options.
What is guaranteed by this PR is narrower:
Providing full concurrent isolation for imperative table reads would require a larger architectural change, such as separating render-local options from committed options or returning a render-specific table facade.
External atom identities are also expected to remain construction-stable, consistent with the current options merging behavior.
This pull request introduces significant improvements to the React adapter for TanStack Table, focusing on more robust and predictable synchronization of controlled state, better integration with React's commit phase, and enhanced test coverage. The main changes ensure that externally controlled state is only published to the table's atom graph after React commits, preventing render-phase errors and improving fine-grained reactivity. Additionally, new tests are added to verify correct behavior, and dependency updates ensure compatibility with React 19.
React Adapter and State Synchronization Enhancements:
packages/react-table/src/reactivity.ts,packages/react-table/src/useTable.ts,packages/table-core/src/core/table/coreTablesFeature.utils.ts,packages/table-core/src/core/table/constructTable.ts: Refactored the adapter to synchronize controlled state into the table's atom graph only after React commits, using a newcommitmethod and a customuseTableSelectorhook. This prevents render-phase errors and ensures that derived atoms and table APIs always read a consistent snapshot. The synchronization now supports an explicit comparator and is controlled via a newsyncExternalStateoption. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]Test Coverage Improvements:
examples/react/basic-external-atoms/tests/e2e/smoke.spec.ts,examples/react/basic-external-state/tests/e2e/smoke.spec.ts: Added new end-to-end tests to verify that table updates correctly reflect changes from external atoms without requiring outer React subscriptions and that controlled pagination updates do not cause render-phase errors. [1] [2]packages/react-table/tests/test-setup.ts,packages/react-table/vite.config.ts: Ensured the test environment is properly set up for React 19 by enabling the act environment and including the setup file in Vite config. [1] [2]Dependency Updates:
packages/react-table/package.json: Addedreact-domand@types/react-domas dev dependencies to support React 19 features and testing.Example and Usage Updates:
examples/react/basic-external-atoms/src/main.tsx: Updated the example to remove unnecessaryuseSelectorcalls, relying on the improved React adapter for atom subscriptions and state access. [1] [2] [3] [4] [5]Summary by CodeRabbit