Skip to content

fix: react adapter sync state - #6458

Open
riccardoperra wants to merge 2 commits into
betafrom
fix/react-sync-state-adapter
Open

fix: react adapter sync state#6458
riccardoperra wants to merge 2 commits into
betafrom
fix/react-sync-state-adapter

Conversation

@riccardoperra

@riccardoperra riccardoperra commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

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:

  1. Reading the latest table options and controlled state during React render.
  2. Publishing that state into the reactive atom graph after React commits.

Previously, useTable called setOptions() during render, and setOptions() immediately synchronized controlled state into writable base atoms. Updating those atoms synchronously notified table.store, which could call React’s external-store listener while another render was still in progress:

React render
  -> table.setOptions()
  -> controlled state copied into base atom
  -> table.store synchronously notifies React
  -> React schedules an update during render
  -> "Cannot update a component while rendering a different component"

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:

  • Controlled state passed through options.state.
  • External atoms passed through options.atoms.
  • Internal/uncontrolled table state.
  • Per-slice ownership changes between controlled and internal state.
  • External atoms without requiring an additional outer useSelector.
  • Root-level state selectors without redundant React renders.
  • Isolated table.Subscribe consumers.
  • Stable getSnapshot() references.
  • React Strict Mode.
  • Rapid consecutive updates.
  • Controlled objects recreated on every render.
  • Suspended renders that must not publish uncommitted state.
  • Backward compatibility for non-React adapters and existing static APIs.

State ownership model

Each state slice now resolves using the documented precedence:

options.atoms[key]
  > own options.state[key]
  > baseAtoms[key]

The check against options.state is 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]
Loading

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.state changed.

The React createReadonlyAtom implementation therefore returns a small live facade:

  • get() evaluates the resolver against the latest render-time options.
  • The result is cached using the configured comparator.
  • Repeated reads return a stable reference when the result is semantically unchanged.
  • A hidden computed atom owns the real subscription.
  • The hidden computed tracks normal atom dependencies plus a React commit atom.

Conceptually:

const getSnapshot = () => {
  const next = computeFromCurrentOptions()

  if (!hasSnapshot || !compare(snapshot, next)) {
    snapshot = next
  }

  return snapshot
}

const reactiveAtom = createAtom(() => {
  commitAtom.get()
  return getSnapshot()
})

This gives us two complementary behaviors:

  • Render-time getters always see the current controlled options.
  • Subscribers are invalidated only by real reactive writes or by a React commit.

2. Render phase: update options without publishing atoms

useTable now updates the table options using:

table_setOptions(table, updater, {
  syncExternalState: false,
})

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.state contains the new value.
  • table.atoms.<slice>.get() returns the new value.
  • table.store.get() returns the new value.
  • table.state returns the selected new value.
  • Row models can use the new value.
  • No writable atom is updated.
  • No store subscriber is notified during render.

3. Commit phase: publish the captured state

After React commits, an isomorphic layout effect publishes the controlled state captured by that render:

reactivity.batch(() => {
  table_syncExternalStateToBaseAtoms(
    table,
    capturedControlledState,
    shallow,
  )

  reactivity.commit()
})

Using the captured state is important. The synchronization must represent the render that actually committed, rather than rereading a potentially newer mutable table.options object.

The operation is batched so that:

  • Controlled slices are mirrored into base atoms.
  • Semantically equal slices do not cause redundant writes.
  • The React commit atom invalidates readonly atoms.
  • Subscribers observe one coherent snapshot.
  • Isolated subscribers can update before paint.

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.store must continue notifying its subscribers after a commit. This is required for consumers such as:

<table.Subscribe selector={(state) => state.pagination.pageIndex}>
  {(pageIndex) => <span>{pageIndex}</span>}
</table.Subscribe>

However, the root useTable component has already rendered the new controlled value. Forwarding the same publication to React’s root external-store listener would be redundant.

useTableSelector wraps the existing TanStack Store useSelector; it does not reimplement useSyncExternalStoreWithSelector.

It records the selector and selected snapshot that React actually committed. When the public store later publishes:

if (!compare(committedSelection, nextSelection)) {
  onStoreChange(nextState)
}

Therefore:

  • Public store subscribers still receive the update.
  • Isolated table.Subscribe components can update.
  • The root React listener is skipped when the committed selection already matches.
  • Controlled updates produce only the render caused by the controlling React state.
  • Unrelated store changes do not call the root listener when the selected result is unchanged.

Hook ordering is intentional:

  1. useTableSelector records the committed root selection in its layout effect.
  2. useTable publishes the atom graph in its following layout effect.
  3. The filtered root subscription can compare against the correct committed selection.

5. Stable aggregate snapshots

The aggregate table.store readonly 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 result of getSnapshot should be cached to avoid an infinite loop

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 changed
Loading

Supported scenarios

Scenario State source Expected behavior
Internal/uncontrolled state baseAtoms[key] Table APIs update the base atom and notify subscribers immediately. The root rerenders only if its selected value changes.
Controlled state options.state[key] The current render reads the controlled value immediately. Base synchronization and subscriber notification happen only after commit.
External atom options.atoms[key] The table subscribes internally to the atom. No outer React useSelector is required.
External atom and controlled state for the same slice External atom The external atom wins. Controlled state does not override the slice resolver.
Controlled → internal ownership Base atom After ownership is released, the current base value becomes visible. Commit invalidation wakes memoized or isolated subscribers even if no new base write occurs.
Internal → controlled ownership Controlled state The controlled value is visible in the same React render without waiting for atom synchronization.
Unselected slice changes Any source The public store may update, but the root selector does not rerender or forward onStoreChange when its selected result is unchanged.
Recreated controlled objects Controlled state Shallow comparison suppresses equivalent base writes and prevents maximum-depth loops.
Rapid controlled updates Controlled state React resolves the final controlled value; committed publication brings isolated subscribers to the same final snapshot.
Strict Mode Controlled state Repeated render/effect execution remains idempotent and comparator-gated.
Suspended render Controlled state Render-time reads can evaluate the WIP value, but base atoms are not mutated and store subscribers are not notified because no commit effect runs.

API changes

table_setOptions

The function accepts an optional final configuration argument:

table_setOptions(table, updater, {
  syncExternalState: false,
})

The default remains unchanged:

table_setOptions(table, updater)

continues to synchronize external state immediately. This preserves existing behavior for core and other adapters.

table_syncExternalStateToBaseAtoms

The function can now receive:

  • An explicit state snapshot.
  • An optional comparator.
table_syncExternalStateToBaseAtoms(
  table,
  capturedState,
  shallow,
)

Passing an explicit snapshot allows React to publish the state associated with a specific committed render.

The implementation distinguishes:

table_syncExternalStateToBaseAtoms(table)

from:

table_syncExternalStateToBaseAtoms(table, undefined)

The first form reads table.options.state. The second intentionally publishes no controlled state. This prevents an explicitly captured undefined value from accidentally falling back to a newer mutable options object.

Usage examples

Controlled React state

function App() {
  const [pagination, setPagination] = React.useState({
    pageIndex: 0,
    pageSize: 10,
  })

  const table = useTable(
    {
      data,
      columns,
      features,
      state: {
        pagination,
      },
      onPaginationChange: setPagination,
    },
    (state) => state.pagination,
  )

  return (
    <>
      <div>Page {table.state.pageIndex + 1}</div>
      <button onClick={() => table.nextPage()}>Next</button>
    </>
  )
}

The first and subsequent pagination updates now:

  • Render the correct controlled state.
  • Produce the correct paginated row model.
  • Do not synchronously update the atom graph during render.
  • Do not produce the React render-phase warning.
  • Do not cause a redundant root render after commit.

External atom without an outer subscription

function App() {
  const paginationAtom = useCreateAtom({
    pageIndex: 0,
    pageSize: 10,
  })

  const table = useTable(
    {
      data,
      columns,
      features,
      atoms: {
        pagination: paginationAtom,
      },
    },
    (state) => state.pagination,
  )

  return (
    <>
      <div>Page {table.state.pageIndex + 1}</div>
      <button onClick={() => table.nextPage()}>Next</button>
    </>
  )
}

No additional subscription is required:

// Not needed
useSelector(paginationAtom)

The hidden readonly atom tracks the external atom, and useTable reacts through table.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:

  • Suspends.
  • Is abandoned.
  • Is replaced by a newer render.
  • Never commits.

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:

  • It runs only for a committed render.
  • The committed root selection is recorded first.
  • The atom graph is updated before paint.
  • Root notifications can be filtered.
  • Isolated subscribers can observe the committed state without relying on an arbitrary delay.

Test coverage

The focused regression coverage includes:

Area Coverage
Controlled state Live store reads, stable root subscription and no redundant render
Ownership transitions Controlled/internal transitions, including isolated-subscriber invalidation
External atoms Atom updates without an outer useSelector, including row-model changes
Precedence External atom > controlled state > internal base atom
Warning regression No render-phase warning after the first or second pagination update
State consistency React state, table.state, slice atom, flat store, subscriber and row model remain aligned
Selector gating Unselected internal and external changes do not rerender the root
Snapshot stability Stable store.get(), fresh selector objects and recreated controlled slices
Rapid updates Memoized isolated subscribers receive the final controlled value
Commit safety Suspended renders do not mutate base state or notify subscribers
Strict Mode Controlled updates remain stable under repeated rendering/effect execution
Browser integration Controlled-state and external-atoms examples exercise pagination in Chromium

Verified focused results:

  • React adapter tests: 11/11
  • Core atom tests: 11/11
  • Controlled-state Chromium E2E: 3/3
  • External-atoms Chromium E2E: 3/3
  • Focused total: 28/28

Additional validation performed:

  • Full table-core suite: 1005/1005
  • Preact adapter suite: 1/1
  • React/core/preact typechecks: passed
  • React/core/preact builds: passed
  • Controlled-state and external-atoms example builds: passed
  • ESLint, Prettier and diff checks: passed

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.options is still a shared mutable object updated during render. Therefore, while a concurrent render is suspended or later abandoned, imperative reads such as:

table.store.get()
table.atoms.pagination.get()
table.getRowModel()

may observe render-in-progress options.

What is guaranteed by this PR is narrower:

  • A suspended render does not mutate the committed base atoms.
  • A suspended render does not notify store subscribers.
  • Only a committed render publishes controlled state into the reactive graph.

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 new commit method and a custom useTableSelector hook. 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 new syncExternalState option. [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:

Example and Usage Updates:

Summary by CodeRabbit

  • New Features
    • Added React-table helpers for selector-based subscriptions and isomorphic layout effects.
  • Bug Fixes
    • Improved controlled-state and external-state synchronization to prevent render-phase updates and reduce unnecessary re-renders, including more reliable pagination updates after commits.
    • Enhanced behavior when switching between controlled and uncontrolled pagination.
  • Tests
    • Added expanded React hook coverage and new end-to-end smoke tests validating pagination navigation, external atoms, and absence of React update warnings/errors.

@nx-cloud

nx-cloud Bot commented Jul 27, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 46921d5

Command Status Duration Result
nx affected --targets=test:eslint,test:sherif,t... ✅ Succeeded 3m 37s View ↗
nx run-many --targets=build --exclude=examples/** ✅ Succeeded 27s View ↗

☁️ Nx Cloud last updated this comment at 2026-07-27 18:19:35 UTC

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Controlled 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.

Changes

React commit-aligned table reactivity

Layer / File(s) Summary
Controlled state and atom synchronization
packages/table-core/src/core/table/constructTable.ts, packages/table-core/src/core/table/coreTablesFeature.utils.ts, packages/table-core/tests/unit/core/tableAtoms.test.ts
Controlled state now overrides derived atom values while preserving invalidation; synchronization supports explicit snapshots, comparators, and deferred updates.
React reactivity bindings
packages/react-table/src/reactivity.ts, packages/react-table/package.json
React bindings add commit publication, cached snapshots, commit-gated subscriptions, and useTableSelector.
useTable commit synchronization
packages/react-table/src/useTable.ts
useTable updates options without immediate base synchronization, selects state through the React selector, and publishes controlled state in an isomorphic layout effect.
Pagination behavior validation
packages/react-table/tests/useTable.test.tsx, packages/react-table/tests/test-setup.ts, packages/react-table/vite.config.ts, examples/react/basic-external-atoms/src/main.tsx, examples/react/basic-external-atoms/tests/e2e/smoke.spec.ts, examples/react/basic-external-state/tests/e2e/smoke.spec.ts
Unit and end-to-end coverage verifies pagination ownership, external atoms, concurrent rendering, commit publication, and absence of React update errors.

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
Loading

Possibly related PRs

  • TanStack/table#6237: Extends the same table-core external reactivity binding infrastructure used by this change.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing React adapter state synchronization.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/react-sync-state-adapter

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Jul 27, 2026

Copy link
Copy Markdown
More templates

@tanstack/alpine-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/alpine-table@6458

@tanstack/angular-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/angular-table@6458

@tanstack/angular-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/angular-table-devtools@6458

@tanstack/ember-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/ember-table@6458

@tanstack/lit-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/lit-table@6458

@tanstack/match-sorter-utils

npm i https://pkg.pr.new/TanStack/table/@tanstack/match-sorter-utils@6458

@tanstack/preact-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/preact-table@6458

@tanstack/preact-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/preact-table-devtools@6458

@tanstack/react-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/react-table@6458

@tanstack/react-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/react-table-devtools@6458

@tanstack/solid-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/solid-table@6458

@tanstack/solid-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/solid-table-devtools@6458

@tanstack/svelte-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/svelte-table@6458

@tanstack/table-core

npm i https://pkg.pr.new/TanStack/table/@tanstack/table-core@6458

@tanstack/table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/table-devtools@6458

@tanstack/vue-table

npm i https://pkg.pr.new/TanStack/table/@tanstack/vue-table@6458

@tanstack/vue-table-devtools

npm i https://pkg.pr.new/TanStack/table/@tanstack/vue-table-devtools@6458

commit: d0f7a3e

* Isolated `table.Subscribe` consumers still receive the underlying store
* notification; only this hook's root subscription is filtered.
*/
export function useTableSelector<TState, TSelected>(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi this is just a useSyncWithExternalStoreWithSelector revisited implementation

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
packages/react-table/tests/useTable.test.tsx (1)

1-930: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting a shared DOM-query helper.

The container?.querySelector('[data-testid="..."]')?.textContent pattern 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

compare is frozen at first render while selector is kept live via ref.

selectorRef is updated every render so the latest selector is always used, but compare is captured directly by the useState initializer closure and never refreshed — if a caller ever passes a non-stable compare function, this hook would silently keep using the very first one forever. Today this is harmless because both call sites (useTable.ts) pass the same stable shallow reference, 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 value

Skip options.state sync for keys owned by external atoms.

When a slice has both options.atoms[key] and options.state[key], table.atoms[key] reads the external atom only, so table_syncExternalStateToBaseAtoms() leaving baseAtoms[key] stale is safe unless a setter reads table.baseAtoms[key] as the current value. Current feature code reads through table.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

📥 Commits

Reviewing files that changed from the base of the PR and between 0df4675 and 46921d5.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (12)
  • examples/react/basic-external-atoms/src/main.tsx
  • examples/react/basic-external-atoms/tests/e2e/smoke.spec.ts
  • examples/react/basic-external-state/tests/e2e/smoke.spec.ts
  • packages/react-table/package.json
  • packages/react-table/src/reactivity.ts
  • packages/react-table/src/useTable.ts
  • packages/react-table/tests/test-setup.ts
  • packages/react-table/tests/useTable.test.tsx
  • packages/react-table/vite.config.ts
  • packages/table-core/src/core/table/constructTable.ts
  • packages/table-core/src/core/table/coreTablesFeature.utils.ts
  • packages/table-core/tests/unit/core/tableAtoms.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant