From e4b3f568c7b936b7555a839c676f4d33c3dc1ec8 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Mon, 27 Jul 2026 22:34:35 -0500 Subject: [PATCH 1/9] refactor: move render-phase sync strategy into core reactivity bindings Builds on the deferred controlled-state publication from this branch and consolidates it behind a consistent adapter interface: - TableReactivityBindings gains two optional properties describing the two strategy axes: deferExternalStateSync (write-side: is setOptions a notification-safe moment?) and commit (invalidation hook for readonly atoms whose compute reads non-reactive plain options). constructTable warns in dev when defer is set with plain options but no commit hook. - table_setOptions consults the bindings flag instead of a per-call { syncExternalState } option, so an adapter's strategy is declared once. - table_syncExternalStateToBaseAtoms replaces the arguments.length overloads with an explicit `capturedState | null` sentinel, and now owns the commit bump: it runs inside the write batch and fires even when nothing is published, so ownership releases still invalidate subscribers. - New renderPhaseReactivity preset in @tanstack/table-core/reactivity hosts the live readonly-atom facade + commit atom (moved from the React adapter). Store primitives are injected by the adapter so all atoms share one store instance with user external atoms. Preact can adopt the same preset later. - New createCommitFilteredSource replaces useTableSelector: because facade snapshots are referentially stable, a reference check on the last snapshot read is enough to skip the root hook's redundant post-commit notification. This drops the committed-selection refs and the implicit requirement that the selector's layout effect run before the publish effect. - useTable: plain useSelector over the filtered source; the publish layout effect is a single table_syncExternalStateToBaseAtoms call. All 11 react-table tests from this branch pass unchanged. table-core: 1017 tests, types, lint, size-limit green. Both example e2e smoke suites pass. Verified in basic-external-state (StrictMode + React Compiler + devtools): one render pass and one commit per controlled update, no render-phase warning, correct behavior through pagination/sorting/page-size/1M-row stress. Co-Authored-By: Claude Fable 5 --- packages/react-table/src/reactivity.ts | 150 +--------------- packages/react-table/src/useTable.ts | 73 ++++---- .../reactivity/coreReactivityFeature.types.ts | 24 +++ .../core/reactivity/renderPhaseReactivity.ts | 163 ++++++++++++++++++ .../src/core/table/constructTable.ts | 13 ++ .../src/core/table/coreTablesFeature.utils.ts | 85 +++++---- packages/table-core/src/reactivity.ts | 1 + .../unit/core/renderPhaseReactivity.test.ts | 160 +++++++++++++++++ .../tests/unit/core/tableAtoms.test.ts | 32 ++-- 9 files changed, 470 insertions(+), 231 deletions(-) create mode 100644 packages/table-core/src/core/reactivity/renderPhaseReactivity.ts create mode 100644 packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts diff --git a/packages/react-table/src/reactivity.ts b/packages/react-table/src/reactivity.ts index 1e7cfdbaf9..fdc9e174df 100644 --- a/packages/react-table/src/reactivity.ts +++ b/packages/react-table/src/reactivity.ts @@ -1,150 +1,18 @@ -import { batch, createAtom, useSelector } from '@tanstack/react-store' -import { useEffect, useLayoutEffect, useRef, useState } from 'react' -import type { Subscription } from '@tanstack/react-store' -import type { - TableAtomOptions, - TableReactivityBindings, -} from '@tanstack/table-core/reactivity' +import { batch, createAtom } from '@tanstack/react-store' +import { renderPhaseReactivity } from '@tanstack/table-core/reactivity' +import type { RenderPhaseReactivityBindings } from '@tanstack/table-core/reactivity' -interface ExternalSource { - get: () => T - subscribe: (listener: (value: T) => void) => Subscription -} - -export const useIsomorphicLayoutEffect = - typeof window === 'undefined' ? useEffect : useLayoutEffect - -export interface ReactTableReactivityBindings extends TableReactivityBindings { - /** - * Invalidates readonly atoms after React commits updated plain options. - */ - commit: () => void -} +export type ReactTableReactivityBindings = RenderPhaseReactivityBindings /** * Creates the table-core reactivity bindings used by the React adapter. * * React stores table state in TanStack Store atoms and leaves options as plain - * resolved data because `useTable` synchronizes options during render. + * resolved data because `useTable` synchronizes options during render. The + * render-phase preset supplies the live readonly-atom facades and the `commit` + * hook; the store primitives are passed in from `@tanstack/react-store` so all + * atoms share one store instance with user-provided external atoms. */ export function reactReactivity(): ReactTableReactivityBindings { - const commitAtom = createAtom(0) - - return { - createOptionsStore: false, - wrapExternalAtoms: false, - addSubscription: () => { - throw new Error( - 'Feature not supported in current reactivity implementation', - ) - }, - unmount: () => { - throw new Error( - 'Feature not supported in current reactivity implementation', - ) - }, - schedule: (fn) => queueMicrotask(fn), - batch, - untrack: (fn) => fn(), - createReadonlyAtom: (fn: () => T, options?: TableAtomOptions) => { - const compare = options?.compare ?? Object.is - let hasSnapshot = false - let snapshot: T - - // The returned atom is a live facade: get() must see the options from the - // render in progress, while subscribe() must only publish after commit. - // The hidden computed tracks both real atom dependencies and commitAtom, - // which covers changes whose only reactive source is a plain option - // (controlled state ownership, for example). - const getSnapshot = () => { - const nextSnapshot = fn() - - if (!hasSnapshot || !compare(snapshot, nextSnapshot)) { - snapshot = nextSnapshot - hasSnapshot = true - } - - return snapshot - } - - const reactiveAtom = createAtom( - () => { - commitAtom.get() - return getSnapshot() - }, - { - compare, - }, - ) - - return { - get: getSnapshot, - subscribe: reactiveAtom.subscribe.bind(reactiveAtom), - } - }, - createWritableAtom: (value: T, options?: TableAtomOptions) => { - return createAtom(value, { - compare: options?.compare, - }) - }, - commit: () => { - commitAtom.set((version) => version + 1) - }, - } -} - -/** - * Selects the root table state while filtering notifications that merely - * publish a snapshot already read during the current React render. - * - * Isolated `table.Subscribe` consumers still receive the underlying store - * notification; only this hook's root subscription is filtered. - */ -export function useTableSelector( - source: ExternalSource, - selector: ((state: TState) => TSelected) | undefined, - compare: (previous: TSelected, next: TSelected) => boolean, -): TSelected { - const committedSelectorRef = useRef(selector) - const committedSelectionRef = useRef< - | { hasSnapshot: false } - | { - hasSnapshot: true - snapshot: TSelected - } - >({ hasSnapshot: false }) - - const [filteredSource] = useState(() => { - return { - get: () => source.get(), - subscribe: (onStoreChange: (value: TState) => void) => { - return source.subscribe((nextState) => { - const select = - committedSelectorRef.current ?? - ((state: TState) => state as unknown as TSelected) - const nextSelection = select(nextState) - const committedSelection = committedSelectionRef.current - - if ( - !committedSelection.hasSnapshot || - !compare(committedSelection.snapshot, nextSelection) - ) { - onStoreChange(nextState) - } - }) - }, - } - }) - - const selected = useSelector(filteredSource, selector, { compare }) - - useIsomorphicLayoutEffect(() => { - committedSelectorRef.current = selector - committedSelectionRef.current = { - hasSnapshot: true, - snapshot: selected, - } - }) - - return selected + return renderPhaseReactivity({ createAtom, batch }) } diff --git a/packages/react-table/src/useTable.ts b/packages/react-table/src/useTable.ts index b5ae06d637..6da8126cb1 100644 --- a/packages/react-table/src/useTable.ts +++ b/packages/react-table/src/useTable.ts @@ -1,17 +1,14 @@ 'use client' -import { useMemo, useState } from 'react' +import { useEffect, useLayoutEffect, useMemo, useState } from 'react' import { constructTable } from '@tanstack/table-core' +import { createCommitFilteredSource } from '@tanstack/table-core/reactivity' import { table_setOptions, table_syncExternalStateToBaseAtoms, } from '@tanstack/table-core/static-functions' -import { shallow } from '@tanstack/react-store' -import { - reactReactivity, - useIsomorphicLayoutEffect, - useTableSelector, -} from './reactivity' +import { shallow, useSelector } from '@tanstack/react-store' +import { reactReactivity } from './reactivity' import { FlexRender } from './FlexRender' import { Subscribe } from './Subscribe' import type { FlexRenderProps } from './FlexRender' @@ -26,6 +23,9 @@ import type { } from '@tanstack/table-core' import type { FunctionComponent, ReactNode } from 'react' +const useIsomorphicLayoutEffect = + typeof window === 'undefined' ? useEffect : useLayoutEffect + export type ReactTable< TFeatures extends TableFeatures, TData extends RowData, @@ -154,16 +154,14 @@ export function useTable< tableOptions: TableOptions, selector?: (state: TableState) => TSelected, ): ReactTable { - const [{ table, reactivity }] = useState(() => { - const reactivity = reactReactivity() - + const [{ table, rootSource }] = useState(() => { // Explicit type arguments skip generic inference from the spread object (a // type-check hot spot); the spread only adds the react reactivity binding // to `features`. const tableInstance = constructTable({ ...tableOptions, features: { - coreReactivityFeature: reactivity, + coreReactivityFeature: reactReactivity(), ...tableOptions.features, }, }) as unknown as ReactTable @@ -181,37 +179,46 @@ export function useTable< return { table: tableInstance, - reactivity, + // The commit publication below re-notifies `table.store` so isolated + // `table.Subscribe` consumers update, but this root hook already + // rendered that exact snapshot — forwarding the notification here would + // re-render the owner once per controlled update just to find nothing + // changed. Only this hook's subscription is filtered. + rootSource: createCommitFilteredSource>( + tableInstance.store, + ), } }) const coreTable = table as unknown as Table - // Keep non-state options current during render, but publish controlled state - // into the subscribed atom graph only after React commits. - table_setOptions( - coreTable, - (prev) => ({ - ...prev, - ...tableOptions, - }), - { - syncExternalState: false, - }, - ) + // Keep options current during render, so render reads (data, columns, + // callbacks, controlled state) never lag a frame behind. The reactivity + // bindings declare `deferExternalStateSync`, so no store subscriber is + // notified while React is still rendering; the readonly atoms expose the + // fresh controlled state through their live get() in the meantime. + table_setOptions(coreTable, (prev) => ({ + ...prev, + ...tableOptions, + })) + // Capture this render's controlled state: `table.options` is a shared + // mutable object, and by the time the effect runs it may hold values from a + // newer render that never commits (e.g. a suspended transition). const controlledState = coreTable.options.state - const state = useTableSelector(table.store, selector, shallow) + + const state = useSelector(rootSource, selector, { compare: shallow }) useIsomorphicLayoutEffect(() => { - // Publish only the state captured by a committed render. The React - // readonly atoms already expose it through their live get(). Invalidating - // in the same pre-paint batch also updates isolated table.Subscribe - // consumers and handles changes in controlled/uncontrolled ownership. - reactivity.batch(() => { - table_syncExternalStateToBaseAtoms(coreTable, controlledState, shallow) - reactivity.commit() - }) + // Publish only the state captured by a committed render. Layout effect + // (not passive) so isolated table.Subscribe consumers update before + // paint. Core batches the writes and bumps the commit version, which also + // handles changes in controlled/uncontrolled ownership. + table_syncExternalStateToBaseAtoms( + coreTable, + controlledState ?? null, + shallow, + ) }) // we know this is not the most efficient way to return the table, diff --git a/packages/table-core/src/core/reactivity/coreReactivityFeature.types.ts b/packages/table-core/src/core/reactivity/coreReactivityFeature.types.ts index c69b74b239..7700c0da75 100644 --- a/packages/table-core/src/core/reactivity/coreReactivityFeature.types.ts +++ b/packages/table-core/src/core/reactivity/coreReactivityFeature.types.ts @@ -22,6 +22,30 @@ export interface TableAtomOptions extends AtomOptions { export interface TableReactivityBindings { createOptionsStore: boolean wrapExternalAtoms: boolean + /** + * Write-timing strategy. When `true`, `table_setOptions` does NOT mirror + * `options.state` into the base atoms; the adapter syncs options during its + * host framework's render phase (where store notifications are illegal or + * wasteful) and publishes the captured controlled state after the host + * commits by calling `table_syncExternalStateToBaseAtoms` itself. + * + * Leave unset (or `false`) for fine-grained adapters whose `setOptions` runs + * in a write-safe reactive context (effects, watchers) — eager sync is + * correct there and keeps same-tick read-after-write consistency. + */ + deferExternalStateSync?: boolean + /** + * Invalidates readonly atoms whose compute reads non-reactive inputs (plain + * `table.options`). Core calls this at the end of + * `table_syncExternalStateToBaseAtoms` — including when nothing was + * published — so resolution changes with no atom write (e.g. a controlled + * slice released back to internal ownership) still reach subscribers. + * + * Required when `deferExternalStateSync` is `true` and `createOptionsStore` + * is `false`: with plain options and deferred publication there is no other + * invalidation source. Adapters with a reactive options store never need it. + */ + commit?: () => void addSubscription: (subscription: Subscription) => void /** * Creates a writable atom with an initial value. diff --git a/packages/table-core/src/core/reactivity/renderPhaseReactivity.ts b/packages/table-core/src/core/reactivity/renderPhaseReactivity.ts new file mode 100644 index 0000000000..9b7eadf61b --- /dev/null +++ b/packages/table-core/src/core/reactivity/renderPhaseReactivity.ts @@ -0,0 +1,163 @@ +import type { Atom, AtomOptions, ReadonlyAtom } from '@tanstack/store' +import type { + TableAtomOptions, + TableReactivityBindings, +} from './coreReactivityFeature.types' + +/** + * Reactivity bindings for adapters whose options are plain values synchronized + * during the host framework's render phase, with a guaranteed `commit` hook. + */ +export interface RenderPhaseReactivityBindings extends TableReactivityBindings { + commit: () => void +} + +/** + * Store primitives supplied by the adapter. + * + * They MUST come from the adapter's own store package (e.g. + * `@tanstack/react-store`) rather than table-core's copy: dependency tracking + * and batching share module-global state, so atoms created here must live in + * the same store instance as user-provided external atoms and adapter + * subscriptions. + */ +export interface RenderPhaseReactivityPrimitives { + createAtom: { + (getValue: (prev?: T) => T, options?: AtomOptions): ReadonlyAtom + (initialValue: T, options?: AtomOptions): Atom + } + batch: (fn: () => void) => void + /** + * Overrides the deferred-scheduling primitive (defaults to + * `queueMicrotask`). + */ + schedule?: (fn: () => void) => void +} + +/** + * Creates reactivity bindings for render-phase adapters (React, Preact): + * frameworks with plain, non-reactive options that are re-synchronized during + * component render, where store notifications must not fire until the host + * commits. + * + * Readonly atoms are exposed as live facades. `get()` re-evaluates the + * resolver against the options of the render in progress — a normal computed + * cannot know that plain `options.state` changed — and caches the result + * through the configured comparator so external-store consumers (e.g. React's + * `useSyncExternalStore`) see referentially stable snapshots. `subscribe()` + * goes through a hidden computed that tracks the resolver's real atom + * dependencies plus a commit version, so subscribers are invalidated only by + * actual reactive writes or by `commit()` after the host framework commits. + * + * Sets `deferExternalStateSync`, so `table_setOptions` leaves base-atom + * publication to the adapter's post-commit + * `table_syncExternalStateToBaseAtoms` call (which invokes `commit` itself). + * + * @example + * ```ts + * import { batch, createAtom } from '@tanstack/react-store' + * + * export const reactReactivity = () => + * renderPhaseReactivity({ createAtom, batch }) + * ``` + */ +export function renderPhaseReactivity( + primitives: RenderPhaseReactivityPrimitives, +): RenderPhaseReactivityBindings { + const { createAtom, batch } = primitives + const commitAtom = createAtom(0) + + return { + createOptionsStore: false, + wrapExternalAtoms: false, + deferExternalStateSync: true, + addSubscription: () => { + throw new Error( + 'Feature not supported in current reactivity implementation', + ) + }, + unmount: () => { + throw new Error( + 'Feature not supported in current reactivity implementation', + ) + }, + schedule: primitives.schedule ?? ((fn) => queueMicrotask(fn)), + batch, + untrack: (fn) => fn(), + createReadonlyAtom: (fn: () => T, atomOptions?: TableAtomOptions) => { + const compare = atomOptions?.compare ?? Object.is + let hasSnapshot = false + let snapshot: T + + const getSnapshot = () => { + const nextSnapshot = fn() + + if (!hasSnapshot || !compare(snapshot, nextSnapshot)) { + snapshot = nextSnapshot + hasSnapshot = true + } + + return snapshot + } + + const reactiveAtom = createAtom( + () => { + commitAtom.get() + return getSnapshot() + }, + { compare }, + ) + + return { + get: getSnapshot, + subscribe: reactiveAtom.subscribe.bind(reactiveAtom), + } + }, + createWritableAtom: (value: T, atomOptions?: TableAtomOptions) => { + return createAtom(value, { + compare: atomOptions?.compare, + }) + }, + commit: () => { + commitAtom.set((version) => version + 1) + }, + } +} + +/** + * Wraps a store-shaped source so notifications that merely republish the + * snapshot the consumer has already read through `get()` are dropped. + * + * Render-phase adapters publish controlled state after the host framework + * commits so isolated subscribers update, but the component that owns the + * table already rendered that exact snapshot — forwarding the notification to + * its root subscription would make the host re-render it once per controlled + * update just to find nothing changed. Readonly atoms return referentially + * stable snapshots while semantically unchanged, so a reference check + * suffices. + * + * Only the returned source is filtered; other subscribers of the underlying + * store still receive every notification. + */ +export function createCommitFilteredSource(source: { + get: () => T + subscribe: (listener: (value: T) => void) => { unsubscribe: () => void } +}): { + get: () => T + subscribe: (listener: (value: T) => void) => { unsubscribe: () => void } +} { + let lastSeenSnapshot: T | undefined + + return { + get: () => { + lastSeenSnapshot = source.get() + return lastSeenSnapshot + }, + subscribe: (listener) => + source.subscribe((value) => { + if (value !== lastSeenSnapshot) { + listener(value) + } + }), + } +} diff --git a/packages/table-core/src/core/table/constructTable.ts b/packages/table-core/src/core/table/constructTable.ts index d7b4a41ac8..4c8610724e 100644 --- a/packages/table-core/src/core/table/constructTable.ts +++ b/packages/table-core/src/core/table/constructTable.ts @@ -36,6 +36,19 @@ export function constructTable< >(tableOptions: TableOptions): Table { const _reactivity = tableOptions.features.coreReactivityFeature! + if ( + process.env.NODE_ENV === 'development' && + _reactivity.deferExternalStateSync && + !_reactivity.createOptionsStore && + !_reactivity.commit + ) { + console.warn( + 'TanStack Table: reactivity bindings declare `deferExternalStateSync` with plain (non-reactive) options but no `commit` hook. ' + + 'Without a reactive options store or a commit hook, controlled-state resolution changes cannot invalidate subscribers. ' + + 'Provide `commit` (see `renderPhaseReactivity`) or a reactive options store.', + ) + } + // Strip the non-feature slots: type-only meta slots, row model factories, // and row model fn registries all live on the `features` option but are not // table features themselves. diff --git a/packages/table-core/src/core/table/coreTablesFeature.utils.ts b/packages/table-core/src/core/table/coreTablesFeature.utils.ts index 977508837e..8ce1ed2044 100644 --- a/packages/table-core/src/core/table/coreTablesFeature.utils.ts +++ b/packages/table-core/src/core/table/coreTablesFeature.utils.ts @@ -8,61 +8,59 @@ import type { TableState } from '../../types/TableState' /** * Synchronizes externally controlled state slices into the table's base atoms. * - * This keeps legacy `options.state` values reflected in the atom graph so - * derived atoms, stores, and table APIs read a consistent snapshot. - * Passing an explicit state snapshot lets framework adapters publish the state - * captured by a committed render. An optional comparator can suppress - * semantically unchanged slice writes; the default remains `Object.is`. + * This keeps `options.state` values mirrored in the atom graph so derived + * atoms, stores, and table APIs read a consistent snapshot. + * + * Adapters that update options during their host's render phase pass the + * state snapshot captured by the committed render as `capturedState` — the + * shared options object may already hold values from a newer render that + * never commits. Pass `null` to publish nothing (a captured "no controlled + * state"); omitting the argument reads the current `table.options.state` + * instead. An optional `compare` suppresses semantically unchanged slice + * writes; the default remains reference equality. + * + * The reactivity `commit` hook (when bound) is invoked inside the same batch, + * even when nothing is published, so resolution changes with no atom write + * (e.g. a controlled slice released back to internal ownership) still + * invalidate subscribers. * * @example * ```ts * table_syncExternalStateToBaseAtoms(table) + * table_syncExternalStateToBaseAtoms(table, capturedState ?? null, shallow) * ``` */ -export function table_syncExternalStateToBaseAtoms< - TFeatures extends TableFeatures, - TData extends RowData, ->(table: Table_Internal): void export function table_syncExternalStateToBaseAtoms< TFeatures extends TableFeatures, TData extends RowData, >( table: Table_Internal, - state: Partial> | undefined, - compare?: (currentState: unknown, externalState: unknown) => boolean, -): void -export function table_syncExternalStateToBaseAtoms< - TFeatures extends TableFeatures, - TData extends RowData, ->( - table: Table_Internal, - state?: Partial>, - compare: ( - currentState: unknown, - externalState: unknown, - ) => boolean = Object.is, + capturedState?: Partial> | null, + compare: (currentState: unknown, externalState: unknown) => boolean = ( + currentState, + externalState, + ) => currentState === externalState, ): void { - // The second overload intentionally distinguishes an omitted argument from - // an explicitly captured `undefined` state. - state = arguments.length === 1 ? table.options.state : state - - if (!state) { - return - } + const state = + capturedState === undefined ? table.options.state : capturedState table._reactivity.batch(() => { - for (const key in state) { - const baseAtom = (table.baseAtoms as Record)[key] - if (!baseAtom) { - continue - } + if (state) { + for (const key in state) { + const baseAtom = (table.baseAtoms as Record)[key] + if (!baseAtom) { + continue + } - const externalState = state[key as keyof typeof state] - const currentState = table._reactivity.untrack(() => baseAtom.get()) - if (!compare(currentState, externalState)) { - baseAtom.set(() => externalState) + const externalState = state[key as keyof typeof state] + const currentState = table._reactivity.untrack(() => baseAtom.get()) + if (!compare(currentState, externalState)) { + baseAtom.set(() => externalState) + } } } + + table._reactivity.commit?.() }) } @@ -171,10 +169,14 @@ export function table_mergeOptions< * The updater receives the current resolved options and the merged result is * immediately assigned to the table instance. * + * When the adapter's reactivity bindings declare `deferExternalStateSync`, + * controlled state is NOT mirrored into the base atoms here — the adapter + * publishes the captured state after its host framework commits by calling + * `table_syncExternalStateToBaseAtoms` itself. + * * @example * ```ts * table_setOptions(table, (old) => old) - * table_setOptions(table, (old) => old, { syncExternalState: false }) * ``` */ export function table_setOptions< @@ -183,9 +185,6 @@ export function table_setOptions< >( table: Table_Internal, updater: Updater>, - options?: { - syncExternalState?: boolean - }, ): void { const newOptions = functionalUpdate( updater, @@ -198,7 +197,7 @@ export function table_setOptions< } else { table.options = mergedOptions } - if (options?.syncExternalState !== false) { + if (!table._reactivity.deferExternalStateSync) { table_syncExternalStateToBaseAtoms(table) } } diff --git a/packages/table-core/src/reactivity.ts b/packages/table-core/src/reactivity.ts index 83329678f6..ca7a98ddbe 100644 --- a/packages/table-core/src/reactivity.ts +++ b/packages/table-core/src/reactivity.ts @@ -1,2 +1,3 @@ export * from './core/reactivity/coreReactivityFeature.types' export * from './core/reactivity/coreReactivityFeature.utils' +export * from './core/reactivity/renderPhaseReactivity' diff --git a/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts b/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts new file mode 100644 index 0000000000..dd7b663f6b --- /dev/null +++ b/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts @@ -0,0 +1,160 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { batch, createAtom } from '@tanstack/store' +import { constructTable, rowSortingFeature } from '../../../src' +import { + createCommitFilteredSource, + renderPhaseReactivity, +} from '../../../src/reactivity' +import { + table_setOptions, + table_syncExternalStateToBaseAtoms, +} from '../../../src/static-functions' +import { testFeatures } from '../../fixtures/features' +import type { SortingState, Table_Internal } from '../../../src' + +const features = testFeatures({ + rowSortingFeature, +}) + +function makeDeferredTable() { + const bindings = renderPhaseReactivity({ createAtom, batch }) + const table = constructTable({ + features: { + ...features, + coreReactivityFeature: bindings, + }, + columns: [], + data: [], + }) + + return { + bindings, + table, + internalTable: table as unknown as Table_Internal, + } +} + +afterEach(() => { + vi.unstubAllEnvs() + vi.restoreAllMocks() +}) + +describe('renderPhaseReactivity bindings', () => { + it('invokes the commit hook on publish, including when nothing is published', () => { + const { bindings, internalTable } = makeDeferredTable() + const commitSpy = vi.spyOn(bindings, 'commit') + + table_syncExternalStateToBaseAtoms(internalTable, { + sorting: [{ id: 'a', desc: false }], + }) + expect(commitSpy).toHaveBeenCalledTimes(1) + + // A captured "no controlled state" publishes nothing but still bumps the + // commit version so ownership releases invalidate subscribers. + table_syncExternalStateToBaseAtoms(internalTable, null) + expect(commitSpy).toHaveBeenCalledTimes(2) + }) + + it('treats null capturedState as "publish nothing" and omitted as "read current options"', () => { + const { table, internalTable } = makeDeferredTable() + const controlled: SortingState = [{ id: 'controlled', desc: false }] + + table_setOptions(internalTable, (options) => ({ + ...options, + state: { sorting: controlled }, + })) + + table_syncExternalStateToBaseAtoms(internalTable, null) + expect(table.baseAtoms.sorting.get()).toEqual([]) + + table_syncExternalStateToBaseAtoms(internalTable) + expect(table.baseAtoms.sorting.get()).toBe(controlled) + }) + + it('suppresses semantically equal slice writes through the compare parameter', () => { + const { table, internalTable } = makeDeferredTable() + const sorting: SortingState = [{ id: 'a', desc: false }] + + table_syncExternalStateToBaseAtoms(internalTable, { sorting }) + const published = table.baseAtoms.sorting.get() + + const recreated = [...sorting] + table_syncExternalStateToBaseAtoms( + internalTable, + { sorting: recreated }, + (currentState, externalState) => + JSON.stringify(currentState) === JSON.stringify(externalState), + ) + + expect(table.baseAtoms.sorting.get()).toBe(published) + }) + + it('warns in development when deferExternalStateSync lacks both a reactive options store and a commit hook', () => { + vi.stubEnv('NODE_ENV', 'development') + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const incoherent = { + ...renderPhaseReactivity({ createAtom, batch }), + commit: undefined, + } + constructTable({ + features: { + ...features, + coreReactivityFeature: incoherent, + }, + columns: [], + data: [], + }) + + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0]?.[0]).toContain('deferExternalStateSync') + }) +}) + +describe('createCommitFilteredSource', () => { + it('drops notifications whose snapshot the consumer already read, forwards the rest', () => { + const listeners: Array<(value: { n: number }) => void> = [] + let snapshot = { n: 1 } + const source = { + get: () => snapshot, + subscribe: (listener: (value: { n: number }) => void) => { + listeners.push(listener) + return { unsubscribe: () => {} } + }, + } + + const filtered = createCommitFilteredSource(source) + const received: Array<{ n: number }> = [] + filtered.subscribe((value) => received.push(value)) + + // Never read through the filtered source yet — forwarded. + listeners[0]!(snapshot) + expect(received).toEqual([{ n: 1 }]) + + // Read it, then republish the same reference — dropped. + expect(filtered.get()).toBe(snapshot) + listeners[0]!(snapshot) + expect(received).toHaveLength(1) + + // A new snapshot reference is forwarded again. + snapshot = { n: 2 } + listeners[0]!(snapshot) + expect(received).toEqual([{ n: 1 }, { n: 2 }]) + }) + + it('does not filter other subscribers of the underlying source', () => { + const atom = createAtom({ n: 0 }) + const filtered = createCommitFilteredSource(atom) + + const direct: Array = [] + atom.subscribe((value) => direct.push(value.n)) + + filtered.get() + const next = { n: 1 } + atom.set(next) + filtered.get() + atom.set({ n: 2 }) + + expect(direct).toEqual([1, 2]) + }) +}) diff --git a/packages/table-core/tests/unit/core/tableAtoms.test.ts b/packages/table-core/tests/unit/core/tableAtoms.test.ts index 23191ef22c..cd9cffc048 100644 --- a/packages/table-core/tests/unit/core/tableAtoms.test.ts +++ b/packages/table-core/tests/unit/core/tableAtoms.test.ts @@ -1,11 +1,12 @@ import { describe, expect, it, vi } from 'vitest' -import { createAtom } from '@tanstack/store' +import { batch, createAtom } from '@tanstack/store' import { constructTable, rowPaginationFeature, rowSelectionFeature, rowSortingFeature, } from '../../../src' +import { renderPhaseReactivity } from '../../../src/reactivity' import { table_setOptions, table_syncExternalStateToBaseAtoms, @@ -73,27 +74,30 @@ describe('three-layer atom architecture', () => { expect(table.store.state.sorting).toEqual(external) }) - it('can defer options.state synchronization for framework adapters', () => { - const table = makeTable() + it('defers options.state synchronization when bindings declare deferExternalStateSync', () => { + const table = constructTable({ + features: { + ...features, + coreReactivityFeature: renderPhaseReactivity({ createAtom, batch }), + }, + columns: [], + data: [], + }) const internalTable = table as unknown as Table_Internal< typeof features, any > const controlled: SortingState = [{ id: 'controlled', desc: false }] - table_setOptions( - internalTable, - (options) => ({ - ...options, - state: { - sorting: controlled, - }, - }), - { - syncExternalState: false, + table_setOptions(internalTable, (options) => ({ + ...options, + state: { + sorting: controlled, }, - ) + })) + // Options are current and render reads resolve the controlled value, + // but nothing was published into the base atoms yet. expect(table.options.state?.sorting).toBe(controlled) expect(table.baseAtoms.sorting.get()).toEqual([]) expect(table.atoms.sorting.get()).toBe(controlled) From faadfbd5a2d63574afc5b6e79f454766836f8c69 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 28 Jul 2026 07:10:09 -0500 Subject: [PATCH 2/9] fix: adopt render-phase reactivity preset in the preact adapter Preact had the identical sync-options-during-render code as React and paid the same cost silently: the mid-render store notification scheduled an extra deferred render per controlled update (Preact never warns about it). Same shape as the React adapter: preactReactivity collapses onto renderPhaseReactivity with @tanstack/preact-store primitives, useTable defers publication to an isomorphic layout effect with the captured controlled state, and the root useSelector reads through createCommitFilteredSource. New unit tests assert one render pass per controlled update with consistent controlled/selected/atom/store/row-model reads, exactly one post-commit store notification for external subscribers, and uncontrolled updates still re-rendering. New e2e spec exercises controlled pagination in the basic-external-state preact example. Types, lint, and build green. Co-Authored-By: Claude Fable 5 --- .../tests/e2e/smoke.spec.ts | 34 +++ packages/preact-table/src/reactivity.ts | 46 ++-- packages/preact-table/src/useTable.ts | 53 ++++- .../preact-table/tests/unit/useTable.test.tsx | 200 ++++++++++++++++++ 4 files changed, 293 insertions(+), 40 deletions(-) create mode 100644 packages/preact-table/tests/unit/useTable.test.tsx diff --git a/examples/preact/basic-external-state/tests/e2e/smoke.spec.ts b/examples/preact/basic-external-state/tests/e2e/smoke.spec.ts index 6188138993..d0881ae9dc 100644 --- a/examples/preact/basic-external-state/tests/e2e/smoke.spec.ts +++ b/examples/preact/basic-external-state/tests/e2e/smoke.spec.ts @@ -102,3 +102,37 @@ test('renders the table without crashing', async ({ page }) => { await server.close() } }) + +test('updates controlled pagination without console errors', async ({ + page, +}) => { + const { errors, server } = await openExample(page) + + try { + const table = getTable(page) + const nextPageButton = page.getByRole('button', { + name: '>', + exact: true, + }) + const pageStatus = page.getByText(/^Page$/).locator('..') + + await expect(getBodyRows(table).first()).toBeVisible() + await expect(pageStatus).toContainText('1 of 100') + const firstPageRow = await getFirstBodyRowData(table) + + await nextPageButton.click() + + await expect(pageStatus).toContainText('2 of 100') + await expect.poll(() => getFirstBodyRowData(table)).not.toBe(firstPageRow) + + const secondPageRow = await getFirstBodyRowData(table) + + await nextPageButton.click() + + await expect(pageStatus).toContainText('3 of 100') + await expect.poll(() => getFirstBodyRowData(table)).not.toBe(secondPageRow) + expect(errors).toEqual([]) + } finally { + await server.close() + } +}) diff --git a/packages/preact-table/src/reactivity.ts b/packages/preact-table/src/reactivity.ts index 57ecd811b3..334fa813dc 100644 --- a/packages/preact-table/src/reactivity.ts +++ b/packages/preact-table/src/reactivity.ts @@ -1,43 +1,21 @@ import { batch, createAtom } from '@tanstack/preact-store' -import type { - TableAtomOptions, - TableReactivityBindings, -} from '@tanstack/table-core/reactivity' +import { renderPhaseReactivity } from '@tanstack/table-core/reactivity' +import type { RenderPhaseReactivityBindings } from '@tanstack/table-core/reactivity' + +export type PreactTableReactivityBindings = RenderPhaseReactivityBindings /** * Creates the table-core reactivity bindings used by the Preact adapter. * - * Preact stores table state in TanStack Store atoms and leaves options as plain - * resolved data because `useTable` synchronizes options during render. + * Preact stores table state in TanStack Store atoms and leaves options as + * plain resolved data because `useTable` synchronizes options during render. + * The render-phase preset supplies the live readonly-atom facades and the + * `commit` hook; the store primitives are passed in from + * `@tanstack/preact-store` so all atoms share one store instance with + * user-provided external atoms. */ -export function preactReactivity(): TableReactivityBindings { - return { - createOptionsStore: false, - wrapExternalAtoms: false, - addSubscription: () => { - throw new Error( - 'Feature not supported in current reactivity implementation', - ) - }, - unmount: () => { - throw new Error( - 'Feature not supported in current reactivity implementation', - ) - }, - schedule: (fn) => queueMicrotask(() => fn()), - batch, - untrack: (fn) => fn(), - createReadonlyAtom: (fn: () => T, options?: TableAtomOptions) => { - return createAtom(() => fn(), { - compare: options?.compare, - }) - }, - createWritableAtom: (value: T, options?: TableAtomOptions) => { - return createAtom(value, { - compare: options?.compare, - }) - }, - } +export function preactReactivity(): PreactTableReactivityBindings { + return renderPhaseReactivity({ createAtom, batch }) } // // TOTO - re-explore preact signals for reactivity diff --git a/packages/preact-table/src/useTable.ts b/packages/preact-table/src/useTable.ts index 73c9681d2f..0e849ade33 100644 --- a/packages/preact-table/src/useTable.ts +++ b/packages/preact-table/src/useTable.ts @@ -1,5 +1,10 @@ -import { useMemo, useState } from 'preact/hooks' +import { useEffect, useLayoutEffect, useMemo, useState } from 'preact/hooks' import { constructTable } from '@tanstack/table-core' +import { createCommitFilteredSource } from '@tanstack/table-core/reactivity' +import { + table_setOptions, + table_syncExternalStateToBaseAtoms, +} from '@tanstack/table-core/static-functions' import { shallow, useSelector } from '@tanstack/preact-store' import { preactReactivity } from './reactivity' import { FlexRender } from './FlexRender' @@ -16,6 +21,9 @@ import type { ComponentChildren } from 'preact' import type { FlexRenderProps } from './FlexRender' import type { SubscribePropsWithStore, SubscribeSource } from './Subscribe' +const useIsomorphicLayoutEffect = + typeof window === 'undefined' ? useEffect : useLayoutEffect + export type PreactTable< TFeatures extends TableFeatures, TData extends RowData, @@ -117,7 +125,7 @@ export function useTable< tableOptions: TableOptions, selector?: (state: TableState) => TSelected, ): PreactTable { - const [table] = useState(() => { + const [{ table, rootSource }] = useState(() => { // Explicit type arguments skip generic inference from the spread object (a // type-check hot spot); the spread only adds the preact reactivity binding // to `features`. @@ -140,16 +148,49 @@ export function useTable< tableInstance.FlexRender = FlexRender - return tableInstance + return { + table: tableInstance, + // The commit publication below re-notifies `table.store` so isolated + // `table.Subscribe` consumers update, but this root hook already + // rendered that exact snapshot — forwarding the notification here would + // re-render the owner once per controlled update just to find nothing + // changed. Only this hook's subscription is filtered. + rootSource: createCommitFilteredSource>( + tableInstance.store, + ), + } }) - // sync options on every render - table.setOptions((prev) => ({ + const coreTable = table as unknown as Table + + // Keep options current during render, so render reads (data, columns, + // callbacks, controlled state) never lag a frame behind. The reactivity + // bindings declare `deferExternalStateSync`, so no store subscriber is + // notified while Preact is still rendering; the readonly atoms expose the + // fresh controlled state through their live get() in the meantime. + table_setOptions(coreTable, (prev) => ({ ...prev, ...tableOptions, })) - const state = useSelector(table.store, selector, { compare: shallow }) + // Capture this render's controlled state: `table.options` is a shared + // mutable object, and by the time the effect runs it may hold values from a + // newer render. + const controlledState = coreTable.options.state + + const state = useSelector(rootSource, selector, { compare: shallow }) + + useIsomorphicLayoutEffect(() => { + // Publish only the state captured by a committed render. Layout effect + // (not passive) so isolated table.Subscribe consumers update before + // paint. Core batches the writes and bumps the commit version, which also + // handles changes in controlled/uncontrolled ownership. + table_syncExternalStateToBaseAtoms( + coreTable, + controlledState ?? null, + shallow, + ) + }) return useMemo( () => ({ diff --git a/packages/preact-table/tests/unit/useTable.test.tsx b/packages/preact-table/tests/unit/useTable.test.tsx new file mode 100644 index 0000000000..36c7f658a0 --- /dev/null +++ b/packages/preact-table/tests/unit/useTable.test.tsx @@ -0,0 +1,200 @@ +import { render } from 'preact' +import { useState } from 'preact/hooks' +import { act } from 'preact/test-utils' +import { + createPaginatedRowModel, + rowPaginationFeature, + tableFeatures, +} from '@tanstack/table-core' +import { afterEach, describe, expect, it } from 'vitest' +import { useTable } from '../../src/useTable' +import type { ColumnDef, PaginationState } from '@tanstack/table-core' +import type { PreactTable } from '../../src/useTable' + +const features = tableFeatures({ + rowPaginationFeature, + paginatedRowModel: createPaginatedRowModel(), +}) + +type TestRow = { + id: number +} + +const data: ReadonlyArray = Array.from({ length: 100 }, (_, id) => ({ + id, +})) +const columns: ReadonlyArray> = [] + +let container: HTMLDivElement | undefined + +function mount(ui: preact.ComponentChildren) { + container = document.createElement('div') + document.body.append(container) + act(() => { + render(ui as any, container!) + }) +} + +function clickButton() { + container?.querySelector('button')?.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + }), + ) +} + +afterEach(() => { + if (container) { + act(() => { + render(null, container!) + }) + container.remove() + container = undefined + } +}) + +describe('useTable controlled state', () => { + it('renders each controlled update in a single pass with consistent reads', () => { + const renderSnapshots: Array<{ + controlled: number + selected: number + atom: number + store: number + firstRow: number + }> = [] + + function ControlledPaginationHarness() { + const [pagination, setPagination] = useState({ + pageIndex: 0, + pageSize: 10, + }) + const table = useTable( + { + features, + columns, + data, + state: { + pagination, + }, + onPaginationChange: setPagination, + }, + (state) => state.pagination, + ) + + renderSnapshots.push({ + controlled: pagination.pageIndex, + selected: table.state.pageIndex, + atom: table.atoms.pagination.get().pageIndex, + store: table.store.get().pagination.pageIndex, + firstRow: table.getRowModel().rows[0]?.original.id ?? -1, + }) + + return + } + + mount() + expect(renderSnapshots).toHaveLength(1) + + act(() => { + clickButton() + }) + // One render pass per controlled update: the root subscription must not + // forward the post-commit publication back into the owner. + expect(renderSnapshots).toHaveLength(2) + + act(() => { + clickButton() + }) + expect(renderSnapshots).toHaveLength(3) + + // Controlled prop, selected state, slice atom, aggregate store, and the + // row model agree within every render pass. + expect( + renderSnapshots.every( + ({ controlled, selected, atom, store, firstRow }) => + controlled === selected && + selected === atom && + atom === store && + firstRow === controlled * 10, + ), + ).toBe(true) + expect(renderSnapshots[2]).toMatchObject({ controlled: 2, firstRow: 20 }) + }) + + it('notifies external store subscribers exactly once per controlled update, after commit', () => { + let latestTable: + | PreactTable + | undefined + let harnessRenderCount = 0 + + function PublicationHarness() { + harnessRenderCount++ + const [pagination, setPagination] = useState({ + pageIndex: 0, + pageSize: 10, + }) + const table = useTable( + { + features, + columns, + data, + state: { + pagination, + }, + onPaginationChange: setPagination, + }, + (state) => state.pagination, + ) + latestTable = table + + return + } + + mount() + + const notifications: Array = [] + const subscription = latestTable!.store.subscribe((state) => { + notifications.push(state.pagination.pageIndex) + }) + + act(() => { + clickButton() + }) + + // The unfiltered public store still publishes the committed snapshot for + // isolated consumers, without re-rendering the owner a second time. + expect(notifications).toEqual([1]) + expect(harnessRenderCount).toBe(2) + + subscription.unsubscribe() + }) + + it('still re-renders for uncontrolled internal updates', () => { + let harnessRenderCount = 0 + + function UncontrolledHarness() { + harnessRenderCount++ + const table = useTable( + { + features, + columns, + data, + }, + (state) => state.pagination.pageIndex, + ) + + return + } + + mount() + expect(harnessRenderCount).toBe(1) + expect(container?.querySelector('button')?.textContent).toBe('0') + + act(() => { + clickButton() + }) + + expect(harnessRenderCount).toBe(2) + expect(container?.querySelector('button')?.textContent).toBe('1') + }) +}) From 5cc598f8b8380d972ff6c01a9b1a5893c1626946 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 28 Jul 2026 07:53:15 -0500 Subject: [PATCH 3/9] fix: adopt render-phase reactivity in the lit adapter, un-pin subscribe islands from stale renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lit basic-subscribe e2e regression had two stacked causes: 1. The subscribe directive returned noChange for host-driven re-renders with an unchanged source + selector, so islands only ever updated from store notifications. They previously received one for ANY setOptions call because the aggregate state snapshot was rebuilt without a comparator; the (correct) shallow compare on table.store stopped notifying for options-only changes, pinning islands to stale row models after Regenerate Data. The directive now re-renders islands on every host update and adopts the latest template closure, while subscriptions still drive updates between host renders. 2. Lit was the off-diagonal reactivity case: a reactive options store written during render(), costing a second update cycle per interaction (requestUpdate mid-update from the optionsStore subscription). litReactivity now uses the renderPhaseReactivity preset with @tanstack/lit-store primitives: plain options synced during render, captured controlled state published from hostUpdated(), and the controller's root subscription reads through createCommitFilteredSource. The optionsStore subscription is gone; options changes flow through host renders (lit reactive properties), which 30 of 31 lit examples already use. basic-subscribe was the lone outlier constructing the table once and pushing data via imperative setOptions in updated() — it now re-syncs options per render pass like the rest. Measured on basic-subscribe: Regenerate Data works again with one host update (previously two, with stale islands), zero idle update churn, and a row-selection click still updates only its island with zero host updates. Unit tests: directive/controller suites updated to the new contract plus a hostUpdated publication test (6/6). All 34 lit example e2e suites pass, including the previously failing basic-subscribe. Types, lint, build green. Co-Authored-By: Claude Fable 5 --- examples/lit/basic-subscribe/src/main.ts | 23 ++++--- packages/lit-table/src/TableController.ts | 64 +++++++++++++----- packages/lit-table/src/reactivity.ts | 47 ++++--------- packages/lit-table/src/subscribe-directive.ts | 19 ++++-- .../lit-table/tests/unit/selectorGate.test.ts | 66 ++++++++++++++++--- packages/preact-table/src/reactivity.ts | 64 ------------------ .../src/core/table/constructTable.ts | 13 ---- 7 files changed, 144 insertions(+), 152 deletions(-) diff --git a/examples/lit/basic-subscribe/src/main.ts b/examples/lit/basic-subscribe/src/main.ts index e2d5c1d2ab..554e42d515 100644 --- a/examples/lit/basic-subscribe/src/main.ts +++ b/examples/lit/basic-subscribe/src/main.ts @@ -118,18 +118,25 @@ class LitTableExample extends LitElement { private tableController = new TableController(this) - private table = this.tableController.table( - { + // Options are re-synced on every render pass (like the other examples), so + // reactive inputs such as `this._data` flow into the table in the same + // update that renders them. + private tableOptions() { + return { features, columns, data: this._data, - getRowId: (row) => row.id, + getRowId: (row: Person) => row.id, enableRowSelection: true, atoms: { rowSelection: rowSelectionAtom, }, debugTable: true, - }, + } + } + + private table = this.tableController.table( + this.tableOptions(), () => null, // subscribe to no table state by default; use table.subscribe below ) @@ -141,12 +148,6 @@ class LitTableExample extends LitElement { pagination: state.pagination, }) - protected updated(changedProperties: Map) { - if (changedProperties.has('_data')) { - this.table.setOptions((prev) => ({ ...prev, data: this._data })) - } - } - private renderColumnFilter( context: HeaderContext, ) { @@ -211,6 +212,8 @@ class LitTableExample extends LitElement { } protected render() { + this.table = this.tableController.table(this.tableOptions(), () => null) + return html`
diff --git a/packages/lit-table/src/TableController.ts b/packages/lit-table/src/TableController.ts index 458d156e17..85d8743609 100644 --- a/packages/lit-table/src/TableController.ts +++ b/packages/lit-table/src/TableController.ts @@ -1,4 +1,6 @@ import { constructTable } from '@tanstack/table-core' +import { createCommitFilteredSource } from '@tanstack/table-core/reactivity' +import { table_syncExternalStateToBaseAtoms } from '@tanstack/table-core/static-functions' import { shallow } from '@tanstack/lit-store' import { litReactivity } from './reactivity' import { FlexRender } from './flexRender' @@ -122,8 +124,14 @@ export class TableController< host: ReactiveControllerHost private _table: Table | null = null + private _rootSource?: { + get: () => TableState + subscribe: (listener: (value: TableState) => void) => { + unsubscribe: () => void + } + } private _storeSubscription?: { unsubscribe: () => void } - private _optionsSubscription?: { unsubscribe: () => void } + private _capturedState?: Partial> private _notifier = 0 private _hasSelector = false private _latestSelector?: (state: TableState) => unknown @@ -173,16 +181,34 @@ export class TableController< this._table = constructTable(mergedOptions) + // The commit publication in hostUpdated() re-notifies `table.store` so + // `table.subscribe` islands update, but the host render already read + // that exact snapshot — forwarding the notification here would update + // the host once per controlled change just to find nothing changed. + // Only the controller's own subscription is filtered. + this._rootSource = createCommitFilteredSource>( + this._table.store, + ) + // Set up subscriptions immediately when table is created this._setupSubscriptions() } - // Update options when they change + // Update options when they change. The reactivity bindings declare + // `deferExternalStateSync`, so no store subscriber is notified while the + // host is still rendering; the readonly atoms expose fresh controlled + // state through their live get() in the meantime. Publication happens in + // hostUpdated(). this._table.setOptions((prev) => ({ ...prev, ...tableOptions, })) + // Capture this render's controlled state: `table.options` is a shared + // mutable object, and by the time hostUpdated() runs it may hold values + // from a newer render pass. + this._capturedState = this._table.options.state + // Record the latest selector each render pass and re-baseline what the // store-subscription gate compares against, so renders triggered by // anything else (e.g. a @state change) reset the gate too. The gate lets @@ -193,31 +219,30 @@ export class TableController< | ((state: TableState) => unknown) | undefined this._lastSelected = selector - ? selector(this._table.store.state) + ? selector(this._rootSource!.get()) : undefined // Capture for closure - const tableInstance = this._table + const rootSource = this._rootSource! return { ...this._table, subscribe, FlexRender, get state() { - return (selector?.(tableInstance.store.state) ?? - tableInstance.store.state) as TSelected + return (selector?.(rootSource.get()) ?? rootSource.get()) as TSelected }, } as unknown as LitTable } private _setupSubscriptions() { if (this._table && !this._storeSubscription) { - this._storeSubscription = this._table.store.subscribe(() => { + this._storeSubscription = this._rootSource!.subscribe((state) => { // With a selector, only update the host when the selected state // actually changes (shallow compare). No selector keeps the previous // behavior of updating on every state change. if (this._hasSelector) { - const nextSelected = this._latestSelector!(this._table!.store.state) + const nextSelected = this._latestSelector!(state) if (shallow(this._lastSelected as any, nextSelected as any)) { return } @@ -226,12 +251,6 @@ export class TableController< this._notifier++ this.host.requestUpdate() }) - - // Options changes (e.g. new data) must always re-render. - this._optionsSubscription = this._table.optionsStore!.subscribe(() => { - this._notifier++ - this.host.requestUpdate() - }) } } @@ -239,10 +258,23 @@ export class TableController< this._setupSubscriptions() } + hostUpdated() { + if (!this._table) { + return + } + // Publish the controlled state captured by the committed render so + // `table.subscribe` islands and other store subscribers observe it. Core + // batches the writes and bumps the commit version, which also handles + // changes in controlled/uncontrolled ownership. + table_syncExternalStateToBaseAtoms( + this._table, + this._capturedState ?? null, + shallow, + ) + } + hostDisconnected() { this._storeSubscription?.unsubscribe() this._storeSubscription = undefined - this._optionsSubscription?.unsubscribe() - this._optionsSubscription = undefined } } diff --git a/packages/lit-table/src/reactivity.ts b/packages/lit-table/src/reactivity.ts index fe4403acfc..ef385363d7 100644 --- a/packages/lit-table/src/reactivity.ts +++ b/packages/lit-table/src/reactivity.ts @@ -1,41 +1,20 @@ import { batch, createAtom } from '@tanstack/lit-store' -import type { - TableAtomOptions, - TableReactivityBindings, -} from '@tanstack/table-core/reactivity' +import { renderPhaseReactivity } from '@tanstack/table-core/reactivity' +import type { RenderPhaseReactivityBindings } from '@tanstack/table-core/reactivity' + +export type LitTableReactivityBindings = RenderPhaseReactivityBindings /** * Creates the table-core reactivity bindings used by the Lit adapter. * - * Lit uses TanStack Store atoms directly. `TableController` subscribes to the - * resulting table store and options store to request host updates. + * Lit calls `controller.table(options)` from the host's `render()`, so options + * are plain values synchronized during the update cycle — writing a reactive + * options store there would schedule a second update per interaction. The + * render-phase preset supplies the live readonly-atom facades and the `commit` + * hook; `TableController` publishes captured controlled state from + * `hostUpdated()`. Store primitives come from `@tanstack/lit-store` so all + * atoms share one store instance with user-provided external atoms. */ -export function litReactivity(): TableReactivityBindings { - return { - createOptionsStore: true, - wrapExternalAtoms: false, - addSubscription: () => { - throw new Error( - 'Feature not supported in current reactivity implementation', - ) - }, - unmount: () => { - throw new Error( - 'Feature not supported in current reactivity implementation', - ) - }, - schedule: (fn) => queueMicrotask(() => fn()), - batch, - untrack: (fn) => fn(), - createReadonlyAtom: (fn: () => T, options?: TableAtomOptions) => { - return createAtom(() => fn(), { - compare: options?.compare, - }) - }, - createWritableAtom: (value: T, options?: TableAtomOptions) => { - return createAtom(value, { - compare: options?.compare, - }) - }, - } +export function litReactivity(): LitTableReactivityBindings { + return renderPhaseReactivity({ createAtom, batch }) } diff --git a/packages/lit-table/src/subscribe-directive.ts b/packages/lit-table/src/subscribe-directive.ts index 1cebb77b12..e6db99371e 100644 --- a/packages/lit-table/src/subscribe-directive.ts +++ b/packages/lit-table/src/subscribe-directive.ts @@ -109,6 +109,12 @@ export class SubscribeDirective extends AsyncDirective { const shouldReinitialize = !this.initialized || sourceChanged || selectorChanged + // The template closure is recreated by every host render and captures + // values from the enclosing render scope (the table wrapper, row models), + // so always adopt the latest one — a stale closure would keep rendering + // through the previous render's captures on subscription-driven updates. + this.resolvedTemplate = actualTemplate + if (shouldReinitialize) { if (this.initialized) { this.controller?.hostDisconnected() @@ -117,7 +123,6 @@ export class SubscribeDirective extends AsyncDirective { this.latestSource = source this.latestSelector = selector - this.resolvedTemplate = actualTemplate if (!this.controller) { this.controller = new TanStackStoreSelector( @@ -129,12 +134,16 @@ export class SubscribeDirective extends AsyncDirective { this.controller.hostUpdate() this.initialized = true - - return this.resolvedTemplate?.(this.controller.value) } - // Host rerender with same source + selector, so we can skip updating - return noChange + // Always re-render on host-driven updates, even with an unchanged source + // and selector: the template may read values that changed without a state + // notification (e.g. row models after the host received new data). The + // subscription still drives updates between host renders; skipping here + // would pin the island to stale non-state inputs. + return this.resolvedTemplate?.( + this.latestSelector!(this.latestSource!.get()), + ) } /** Cleans up the controller subscription when the directive is removed from the DOM. */ diff --git a/packages/lit-table/tests/unit/selectorGate.test.ts b/packages/lit-table/tests/unit/selectorGate.test.ts index 3094011704..beed695f58 100644 --- a/packages/lit-table/tests/unit/selectorGate.test.ts +++ b/packages/lit-table/tests/unit/selectorGate.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest' -import { columnSizingFeature } from '@tanstack/table-core' +import { columnSizingFeature, rowSortingFeature } from '@tanstack/table-core' import { TableController } from '../../src/TableController' function createHost() { @@ -71,21 +71,67 @@ describe('TableController selector gate', () => { expect(host.updateCount).toBe(before + 2) }) - test('options changes always update the host, even with an empty selector', () => { + test('options passed to a new render pass are readable in that same pass', () => { const host = createHost() const controller = new TableController(host) + const baseOptions = { + features: { columnSizingFeature }, + columns: [], + data: [] as Array<{ id: number }>, + } + controller.table(baseOptions, () => ({})) + + const before = host.updateCount + // Options changes flow through host renders (lit reactive properties), so + // the render pass that hands in new options reads them immediately — no + // store-driven host update is needed or scheduled. const table = controller.table( - { - features: { columnSizingFeature }, - columns: [], - data: [], - }, + { ...baseOptions, data: [{ id: 1 }] }, () => ({}), ) + expect(table.options.data).toEqual([{ id: 1 }]) + expect(host.updateCount).toBe(before) + }) - const before = host.updateCount - table.setOptions((prev) => ({ ...prev, data: [{ id: 1 }] })) - expect(host.updateCount).toBe(before + 1) + test('hostUpdated publishes captured controlled state to store subscribers', () => { + const host = createHost() + const controller = new TableController(host) + + const initialSorting = [{ id: 'a', desc: false }] + const baseOptions = { + features: { rowSortingFeature }, + columns: [], + data: [], + } + const table = controller.table( + { ...baseOptions, state: { sorting: initialSorting } }, + () => ({}), + ) + + const nextSorting = [{ id: 'b', desc: true }] + const notifications: Array = [] + const subscription = table.store.subscribe((state: any) => { + notifications.push(state.sorting) + }) + + controller.table( + { ...baseOptions, state: { sorting: nextSorting } }, + () => ({}), + ) + + // The render pass reads the new controlled value through the atoms, but + // nothing is published (and no subscriber notified) until the host + // commits. + expect(table.atoms.sorting.get()).toBe(nextSorting) + expect(table.baseAtoms.sorting.get()).toEqual(initialSorting) + expect(notifications).toEqual([]) + + controller.hostUpdated() + + expect(table.baseAtoms.sorting.get()).toBe(nextSorting) + expect(notifications).toEqual([nextSorting]) + + subscription.unsubscribe() }) }) diff --git a/packages/preact-table/src/reactivity.ts b/packages/preact-table/src/reactivity.ts index 334fa813dc..5fecc253a8 100644 --- a/packages/preact-table/src/reactivity.ts +++ b/packages/preact-table/src/reactivity.ts @@ -17,67 +17,3 @@ export type PreactTableReactivityBindings = RenderPhaseReactivityBindings export function preactReactivity(): PreactTableReactivityBindings { return renderPhaseReactivity({ createAtom, batch }) } - -// // TOTO - re-explore preact signals for reactivity -// import { batch, computed, signal, untracked } from '@preact/signals' -// import type { -// TableAtomOptions, -// TableReactivityBindings, -// } from '@tanstack/table-core/reactivity' -// import type { Atom, Observer, ReadonlyAtom } from '@tanstack/preact-store' - -// function observerToCallback( -// observerOrNext: Observer | ((value: T) => void), -// ): (value: T) => void { -// return typeof observerOrNext === 'function' -// ? observerOrNext -// : (value) => observerOrNext.next?.(value) -// } - -// function signalToReadonlyAtom(source: { -// value: T -// subscribe: (observer: (value: T) => void) => () => void -// }): ReadonlyAtom { -// return Object.assign(source, { -// get: () => source.value, -// subscribe: ((observerOrNext: Observer | ((value: T) => void)) => { -// const unsubscribe = source.subscribe(observerToCallback(observerOrNext)) -// return { unsubscribe } -// }) as ReadonlyAtom['subscribe'], -// }) -// } - -// function signalToWritableAtom(source: { -// value: T -// subscribe: (observer: (value: T) => void) => () => void -// }): Atom { -// return Object.assign(source, { -// set: (updater: T | ((prevVal: T) => T)) => { -// source.value = -// typeof updater === 'function' -// ? (updater as (prevVal: T) => T)(source.value) -// : updater -// }, -// get: () => source.value, -// subscribe: ((observerOrNext: Observer | ((value: T) => void)) => { -// const unsubscribe = source.subscribe(observerToCallback(observerOrNext)) -// return { unsubscribe } -// }) as Atom['subscribe'], -// }) -// } - -// export function preactReactivity(): TableReactivityBindings { -// return { -// createReadonlyAtom: (fn: () => T, _options?: TableAtomOptions) => { -// return signalToReadonlyAtom(computed(fn)) -// }, -// createWritableAtom: ( -// value: T, -// _options?: TableAtomOptions, -// ): Atom => { -// return signalToWritableAtom(signal(value)) -// }, -// untrack: untracked, -// batch: batch, -// } -// } diff --git a/packages/table-core/src/core/table/constructTable.ts b/packages/table-core/src/core/table/constructTable.ts index 4c8610724e..d7b4a41ac8 100644 --- a/packages/table-core/src/core/table/constructTable.ts +++ b/packages/table-core/src/core/table/constructTable.ts @@ -36,19 +36,6 @@ export function constructTable< >(tableOptions: TableOptions): Table { const _reactivity = tableOptions.features.coreReactivityFeature! - if ( - process.env.NODE_ENV === 'development' && - _reactivity.deferExternalStateSync && - !_reactivity.createOptionsStore && - !_reactivity.commit - ) { - console.warn( - 'TanStack Table: reactivity bindings declare `deferExternalStateSync` with plain (non-reactive) options but no `commit` hook. ' + - 'Without a reactive options store or a commit hook, controlled-state resolution changes cannot invalidate subscribers. ' + - 'Provide `commit` (see `renderPhaseReactivity`) or a reactive options store.', - ) - } - // Strip the non-feature slots: type-only meta slots, row model factories, // and row model fn registries all live on the `features` option but are not // table features themselves. From dff9ec3dbab547e40cacf422f90671bb1b2feb86 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 28 Jul 2026 07:54:22 -0500 Subject: [PATCH 4/9] test: drop dev-warning assertion to match removed bindings coherence warning The constructTable dev warning for deferExternalStateSync-without-commit was removed in the previous commit; align the test suite (1010 tests green). Co-Authored-By: Claude Fable 5 --- .../unit/core/renderPhaseReactivity.test.ts | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts b/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts index dd7b663f6b..2fcca2d1db 100644 --- a/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts +++ b/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts @@ -35,7 +35,6 @@ function makeDeferredTable() { } afterEach(() => { - vi.unstubAllEnvs() vi.restoreAllMocks() }) @@ -89,26 +88,6 @@ describe('renderPhaseReactivity bindings', () => { expect(table.baseAtoms.sorting.get()).toBe(published) }) - it('warns in development when deferExternalStateSync lacks both a reactive options store and a commit hook', () => { - vi.stubEnv('NODE_ENV', 'development') - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - - const incoherent = { - ...renderPhaseReactivity({ createAtom, batch }), - commit: undefined, - } - constructTable({ - features: { - ...features, - coreReactivityFeature: incoherent, - }, - columns: [], - data: [], - }) - - expect(warn).toHaveBeenCalledTimes(1) - expect(warn.mock.calls[0]?.[0]).toContain('deferExternalStateSync') - }) }) describe('createCommitFilteredSource', () => { From 170db49c8ca0e57001ea67c404274ab5bf50b376 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:56:12 +0000 Subject: [PATCH 5/9] ci: apply automated fixes --- .../table-core/tests/unit/core/renderPhaseReactivity.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts b/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts index 2fcca2d1db..34214ad6fe 100644 --- a/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts +++ b/packages/table-core/tests/unit/core/renderPhaseReactivity.test.ts @@ -87,7 +87,6 @@ describe('renderPhaseReactivity bindings', () => { expect(table.baseAtoms.sorting.get()).toBe(published) }) - }) describe('createCommitFilteredSource', () => { From ac42932b6247704ebf34947201749450e1c071b1 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 28 Jul 2026 09:36:01 -0500 Subject: [PATCH 6/9] consolidate constructTable features loop again --- packages/table-core/src/core/table/constructTable.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/table-core/src/core/table/constructTable.ts b/packages/table-core/src/core/table/constructTable.ts index 2b91ac97f3..be4f335bfa 100644 --- a/packages/table-core/src/core/table/constructTable.ts +++ b/packages/table-core/src/core/table/constructTable.ts @@ -145,7 +145,8 @@ export function constructTable< // invalidation source when it is published after a framework commit. const reactiveState = externalAtom ? externalAtom.get() - : table.baseAtoms[key]!.get() + : // @ts-ignore - looping through stateKeys so we know the key is defined + table.baseAtoms[key].get() if (externalAtom) { return reactiveState @@ -171,7 +172,8 @@ export function constructTable< const snapshot = {} as TableState & TableState_All for (let i = 0; i < stateKeys.length; i++) { const key = stateKeys[i]! - ;(snapshot as Record)[key] = table.atoms[key]!.get() + // @ts-ignore - looping through stateKeys so we know the key is defined + ;(snapshot as Record)[key] = table.atoms[key].get() } return snapshot }, @@ -219,6 +221,7 @@ export function constructTable< rowInstanceInitFns.push(feature.initRowInstanceData.bind(feature)) } feature.initTableInstanceData?.(table) + featuresList[i]!.constructTableAPIs?.(table) } table._cellInstanceInitFns = cellInstanceInitFns @@ -259,9 +262,5 @@ export function constructTable< ) } - for (let i = 0; i < featuresList.length; i++) { - featuresList[i]!.constructTableAPIs?.(table) - } - return table as unknown as Table } From d5a34f6b2605673d9be5b5000c3a88e4b44bd2ec Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 28 Jul 2026 09:48:01 -0500 Subject: [PATCH 7/9] fix: keep the two-phase feature lifecycle in the consolidated features loop The consolidated loop interleaved constructTableAPIs with initTableInstanceData per feature (and ran APIs first within each feature), breaking the lifecycle contract from #6446 that constructTable.test.ts encodes: every feature's table instance data initializes before ANY table APIs are constructed, so API constructors can read instance data owned by other features and init hooks observe a pre-API table. Keep the consolidation of the init-fn collection with instance-data init (one phase-1 loop writing the precomputed arrays directly), and restore constructTableAPIs as its own second pass. table-core: 1097/1097. React/preact/lit adapter suites green. Co-Authored-By: Claude Fable 5 --- .../src/core/table/constructTable.ts | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/packages/table-core/src/core/table/constructTable.ts b/packages/table-core/src/core/table/constructTable.ts index be4f335bfa..fd8ebcb907 100644 --- a/packages/table-core/src/core/table/constructTable.ts +++ b/packages/table-core/src/core/table/constructTable.ts @@ -185,50 +185,46 @@ export function constructTable< ) // pre-compute the init functions to make the other constructors faster - const cellInstanceInitFns: Array< - NonNullable - > = [] - const columnInstanceInitFns: Array< - NonNullable - > = [] - const headerGroupInstanceInitFns: Array< - NonNullable - > = [] - const headerInstanceInitFns: Array< - NonNullable - > = [] - const rowInstanceInitFns: Array< - NonNullable - > = [] + table._cellInstanceInitFns = [] + table._columnInstanceInitFns = [] + table._headerGroupInstanceInitFns = [] + table._headerInstanceInitFns = [] + table._rowInstanceInitFns = [] for (let i = 0; i < featuresList.length; i++) { const feature = featuresList[i]! + // Feature-owned table instance data initializes for EVERY feature before + // ANY table APIs are constructed (second loop below): API constructors may + // read instance data initialized by other features. + feature.initTableInstanceData?.(table) if (feature.initCellInstanceData) { - cellInstanceInitFns.push(feature.initCellInstanceData.bind(feature)) + table._cellInstanceInitFns.push( + feature.initCellInstanceData.bind(feature), + ) } if (feature.initColumnInstanceData) { - columnInstanceInitFns.push(feature.initColumnInstanceData.bind(feature)) + table._columnInstanceInitFns.push( + feature.initColumnInstanceData.bind(feature), + ) } if (feature.initHeaderGroupInstanceData) { - headerGroupInstanceInitFns.push( + table._headerGroupInstanceInitFns.push( feature.initHeaderGroupInstanceData.bind(feature), ) } if (feature.initHeaderInstanceData) { - headerInstanceInitFns.push(feature.initHeaderInstanceData.bind(feature)) + table._headerInstanceInitFns.push( + feature.initHeaderInstanceData.bind(feature), + ) } if (feature.initRowInstanceData) { - rowInstanceInitFns.push(feature.initRowInstanceData.bind(feature)) + table._rowInstanceInitFns.push(feature.initRowInstanceData.bind(feature)) } - feature.initTableInstanceData?.(table) - featuresList[i]!.constructTableAPIs?.(table) } - table._cellInstanceInitFns = cellInstanceInitFns - table._columnInstanceInitFns = columnInstanceInitFns - table._headerGroupInstanceInitFns = headerGroupInstanceInitFns - table._headerInstanceInitFns = headerInstanceInitFns - table._rowInstanceInitFns = rowInstanceInitFns + for (let i = 0; i < featuresList.length; i++) { + featuresList[i]!.constructTableAPIs?.(table) + } if ( process.env.NODE_ENV === 'development' && From ddcd37a065535f8216c3b8214368041643d06bf4 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 28 Jul 2026 09:54:23 -0500 Subject: [PATCH 8/9] refactor!: process feature lifecycle hooks in a single pass Drop the two-phase construction contract (all initTableInstanceData before any constructTableAPIs). Every in-repo hook is pure property assignment with lazy API closures, so the phase barrier bought nothing; features are now processed in one registration-order pass with each feature's instance data initialized just before its own APIs are constructed. New contract, encoded in the lifecycle test and documented in the custom-features guides and TableFeature jsdoc: hooks may rely on data and APIs of features registered earlier; eager cross-feature reads belong in lazy API bodies. table-core 1097/1097; react/preact/lit adapter suites green. Co-Authored-By: Claude Fable 5 --- docs/framework/alpine/guide/custom-features.md | 2 +- docs/framework/angular/guide/custom-features.md | 2 +- docs/framework/ember/guide/custom-features.md | 2 +- docs/framework/lit/guide/custom-features.md | 2 +- docs/framework/preact/guide/custom-features.md | 2 +- docs/framework/react/guide/custom-features.md | 2 +- docs/framework/solid/guide/custom-features.md | 2 +- docs/framework/svelte/guide/custom-features.md | 2 +- docs/framework/vue/guide/custom-features.md | 2 +- packages/table-core/src/core/table/constructTable.ts | 8 +------- packages/table-core/src/types/TableFeatures.ts | 11 +++++++---- .../tests/unit/core/table/constructTable.test.ts | 11 +++++++---- 12 files changed, 24 insertions(+), 24 deletions(-) diff --git a/docs/framework/alpine/guide/custom-features.md b/docs/framework/alpine/guide/custom-features.md index 5d0ad25fcd..8ae8101f19 100644 --- a/docs/framework/alpine/guide/custom-features.md +++ b/docs/framework/alpine/guide/custom-features.md @@ -140,7 +140,7 @@ The `getInitialState` method in a table feature is responsible for setting the d #### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/angular/guide/custom-features.md b/docs/framework/angular/guide/custom-features.md index 50d1fe2e27..5bbfe347ae 100644 --- a/docs/framework/angular/guide/custom-features.md +++ b/docs/framework/angular/guide/custom-features.md @@ -146,7 +146,7 @@ The `getInitialState` method in a table feature is responsible for setting the d ##### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/ember/guide/custom-features.md b/docs/framework/ember/guide/custom-features.md index e303d4b6dc..3141d67095 100644 --- a/docs/framework/ember/guide/custom-features.md +++ b/docs/framework/ember/guide/custom-features.md @@ -146,7 +146,7 @@ The `getInitialState` method in a table feature is responsible for setting the d ##### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/lit/guide/custom-features.md b/docs/framework/lit/guide/custom-features.md index 230212a129..ff65c2f161 100644 --- a/docs/framework/lit/guide/custom-features.md +++ b/docs/framework/lit/guide/custom-features.md @@ -138,7 +138,7 @@ The `getInitialState` method in a table feature is responsible for setting the d #### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/preact/guide/custom-features.md b/docs/framework/preact/guide/custom-features.md index bce4072fb1..536c940018 100644 --- a/docs/framework/preact/guide/custom-features.md +++ b/docs/framework/preact/guide/custom-features.md @@ -146,7 +146,7 @@ The `getInitialState` method in a table feature is responsible for setting the d ##### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/react/guide/custom-features.md b/docs/framework/react/guide/custom-features.md index 42aad2fb41..928547c2a4 100644 --- a/docs/framework/react/guide/custom-features.md +++ b/docs/framework/react/guide/custom-features.md @@ -146,7 +146,7 @@ The `getInitialState` method in a table feature is responsible for setting the d ##### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/solid/guide/custom-features.md b/docs/framework/solid/guide/custom-features.md index 6faf75b019..6350eee0ce 100644 --- a/docs/framework/solid/guide/custom-features.md +++ b/docs/framework/solid/guide/custom-features.md @@ -140,7 +140,7 @@ The `getInitialState` method in a table feature is responsible for setting the d #### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/svelte/guide/custom-features.md b/docs/framework/svelte/guide/custom-features.md index 81edc25879..509d91c658 100644 --- a/docs/framework/svelte/guide/custom-features.md +++ b/docs/framework/svelte/guide/custom-features.md @@ -140,7 +140,7 @@ The `getInitialState` method in a table feature is responsible for setting the d #### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/docs/framework/vue/guide/custom-features.md b/docs/framework/vue/guide/custom-features.md index 862de8122d..c51ed3d4a6 100644 --- a/docs/framework/vue/guide/custom-features.md +++ b/docs/framework/vue/guide/custom-features.md @@ -140,7 +140,7 @@ The `getInitialState` method in a table feature is responsible for setting the d #### initTableInstanceData and resetTableInstanceData -Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Every feature's initialization hook runs before any feature's `constructTableAPIs` hook. +Use `initTableInstanceData` for mutable, non-reactive data that belongs to one table instance, such as an interaction anchor or an imperative cache. It runs once after table options, state atoms, and the store have been created. Features are processed in a single pass in registration order; each feature's initialization hook runs just before that feature's `constructTableAPIs` hook, so hooks may rely on data and APIs from features registered earlier. Use `resetTableInstanceData` to clear that transient data when `table.reset()` runs. Reset hooks run after internally owned table state atoms have been restored to `table.initialState`. They do not reset table state slices or externally controlled state, and `table.reset()` does not rerun `initTableInstanceData`. diff --git a/packages/table-core/src/core/table/constructTable.ts b/packages/table-core/src/core/table/constructTable.ts index fd8ebcb907..dad11f7845 100644 --- a/packages/table-core/src/core/table/constructTable.ts +++ b/packages/table-core/src/core/table/constructTable.ts @@ -193,9 +193,6 @@ export function constructTable< for (let i = 0; i < featuresList.length; i++) { const feature = featuresList[i]! - // Feature-owned table instance data initializes for EVERY feature before - // ANY table APIs are constructed (second loop below): API constructors may - // read instance data initialized by other features. feature.initTableInstanceData?.(table) if (feature.initCellInstanceData) { table._cellInstanceInitFns.push( @@ -220,10 +217,7 @@ export function constructTable< if (feature.initRowInstanceData) { table._rowInstanceInitFns.push(feature.initRowInstanceData.bind(feature)) } - } - - for (let i = 0; i < featuresList.length; i++) { - featuresList[i]!.constructTableAPIs?.(table) + feature.constructTableAPIs?.(table) } if ( diff --git a/packages/table-core/src/types/TableFeatures.ts b/packages/table-core/src/types/TableFeatures.ts index 10fe36c252..50de9d3fbc 100644 --- a/packages/table-core/src/types/TableFeatures.ts +++ b/packages/table-core/src/types/TableFeatures.ts @@ -426,10 +426,13 @@ export interface TableFeature { * instance. * * This runs once during table construction after options, state atoms, and - * the store are available, and before any feature's `constructTableAPIs` - * hook runs. Use `constructTableAPIs` exclusively for assigning table - * methods. Table resets do not rerun this hook; use - * `resetTableInstanceData` to clear transient instance data instead. + * the store are available. Features are processed in a single pass in + * registration order, with each feature's instance data initialized just + * before its own `constructTableAPIs` hook, so this hook may rely on data + * and APIs of features registered earlier. Use `constructTableAPIs` + * exclusively for assigning table methods. Table resets do not rerun this + * hook; use `resetTableInstanceData` to clear transient instance data + * instead. */ initTableInstanceData?: < TFeatures extends TableFeatures, diff --git a/packages/table-core/tests/unit/core/table/constructTable.test.ts b/packages/table-core/tests/unit/core/table/constructTable.test.ts index adecafbfe1..14ba3060ed 100644 --- a/packages/table-core/tests/unit/core/table/constructTable.test.ts +++ b/packages/table-core/tests/unit/core/table/constructTable.test.ts @@ -35,7 +35,7 @@ function getterOnlyMerge(...sources: Array) { } describe('constructTable', () => { - it('initializes feature-owned table data before constructing any table APIs', () => { + it('runs each feature init just before its own API construction, in registration order', () => { const calls: Array = [] const initA = vi.fn((table: any) => { calls.push('init-a') @@ -43,7 +43,9 @@ describe('constructTable', () => { expect(table.baseAtoms.lifecycle.get()).toBe('initial') expect(table.atoms.lifecycle.get()).toBe('initial') expect(table.store.state.lifecycle).toBe('initial') - expect(table.reset).toBeUndefined() + // Hooks may rely on features registered earlier: the core features' + // APIs (like reset) are already constructed by the time this runs. + expect(table.reset).toBeTypeOf('function') table.firstInitialized = true }) const initB = vi.fn((table: any) => { @@ -53,8 +55,9 @@ describe('constructTable', () => { }) const apiA = vi.fn((table: any) => { calls.push('api-a') + // Own feature's instance data is initialized, later features' is not. expect(table.firstInitialized).toBe(true) - expect(table.secondInitialized).toBe(true) + expect(table.secondInitialized).toBeUndefined() }) const apiB = vi.fn((table: any) => { calls.push('api-b') @@ -94,8 +97,8 @@ describe('constructTable', () => { expect(calls).toEqual([ 'init-a', - 'init-b', 'api-a', + 'init-b', 'api-b', 'api-without-init', ]) From f79077eb395cb0261fbe9a76526f34151faa85a8 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 28 Jul 2026 11:03:47 -0500 Subject: [PATCH 9/9] refactor: require precomputed instance-init fn arrays on the table type constructTable always assigns the five _*InstanceInitFns arrays before any constructor can run, so model them as required and drop the non-null assertions at the use sites (including the leftover one in buildHeaderGroups that eslint flagged as unnecessary). Co-Authored-By: Claude Fable 5 --- .../src/core/cells/constructCell.ts | 2 +- .../src/core/columns/constructColumn.ts | 2 +- .../src/core/headers/buildHeaderGroups.ts | 2 +- .../src/core/headers/constructHeader.ts | 2 +- .../table-core/src/core/rows/constructRow.ts | 2 +- .../src/core/table/constructTable.ts | 23 ++++++++----------- .../src/core/table/coreTablesFeature.types.ts | 12 ++++------ 7 files changed, 20 insertions(+), 25 deletions(-) diff --git a/packages/table-core/src/core/cells/constructCell.ts b/packages/table-core/src/core/cells/constructCell.ts index 65c393c938..63e16ac737 100644 --- a/packages/table-core/src/core/cells/constructCell.ts +++ b/packages/table-core/src/core/cells/constructCell.ts @@ -52,7 +52,7 @@ export function constructCell< cell.row = row // Initialize instance-specific data for features that need it - const initFns = table._cellInstanceInitFns! + const initFns = table._cellInstanceInitFns for (let i = 0; i < initFns.length; i++) { initFns[i]!(cell as Cell) } diff --git a/packages/table-core/src/core/columns/constructColumn.ts b/packages/table-core/src/core/columns/constructColumn.ts index e3eb5bfcb8..b26f42f7d6 100644 --- a/packages/table-core/src/core/columns/constructColumn.ts +++ b/packages/table-core/src/core/columns/constructColumn.ts @@ -117,7 +117,7 @@ export function constructColumn< column.parent = parent // Initialize instance-specific data for features that need it - const initFns = table._columnInstanceInitFns! + const initFns = table._columnInstanceInitFns for (let i = 0; i < initFns.length; i++) { initFns[i]!(column as Column) } diff --git a/packages/table-core/src/core/headers/buildHeaderGroups.ts b/packages/table-core/src/core/headers/buildHeaderGroups.ts index ff3391913a..bd913d189e 100644 --- a/packages/table-core/src/core/headers/buildHeaderGroups.ts +++ b/packages/table-core/src/core/headers/buildHeaderGroups.ts @@ -51,7 +51,7 @@ export function buildHeaderGroups< const headerGroups: Array> = [] - const headerGroupInitFns = table._headerGroupInstanceInitFns! + const headerGroupInitFns = table._headerGroupInstanceInitFns const constructHeaderGroup = ( headersToGroup: Array>, diff --git a/packages/table-core/src/core/headers/constructHeader.ts b/packages/table-core/src/core/headers/constructHeader.ts index 6cb5b2b4b3..d8f1bb5361 100644 --- a/packages/table-core/src/core/headers/constructHeader.ts +++ b/packages/table-core/src/core/headers/constructHeader.ts @@ -64,7 +64,7 @@ export function constructHeader< header.subHeaders = [] // Initialize instance-specific data for features that need it - const initFns = table._headerInstanceInitFns! + const initFns = table._headerInstanceInitFns for (let i = 0; i < initFns.length; i++) { initFns[i]!(header as Header) } diff --git a/packages/table-core/src/core/rows/constructRow.ts b/packages/table-core/src/core/rows/constructRow.ts index a950f00505..9fecbb23ed 100644 --- a/packages/table-core/src/core/rows/constructRow.ts +++ b/packages/table-core/src/core/rows/constructRow.ts @@ -59,7 +59,7 @@ export const constructRow = < row.subRows = subRows ?? [] // Initialize instance-specific data (e.g., caches) for features that need it - const initFns = table._rowInstanceInitFns! + const initFns = table._rowInstanceInitFns for (let i = 0; i < initFns.length; i++) { initFns[i]!(row as Row) } diff --git a/packages/table-core/src/core/table/constructTable.ts b/packages/table-core/src/core/table/constructTable.ts index dad11f7845..100bafd7e7 100644 --- a/packages/table-core/src/core/table/constructTable.ts +++ b/packages/table-core/src/core/table/constructTable.ts @@ -58,13 +58,19 @@ export function constructTable< ...features } = tableOptions.features + // pre-compute the init functions to make the other constructors faster const table = { - _reactivity, + _cellInstanceInitFns: [], + _columnInstanceInitFns: [], _features: { ...coreFeatures, ...features }, - _rowModels: {}, + _headerGroupInstanceInitFns: [], + _headerInstanceInitFns: [], + _reactivity, + _rowInstanceInitFns: [], _rowModelFns: { aggregationFns, filterFns, sortFns }, - baseAtoms: {}, + _rowModels: {}, atoms: {}, + baseAtoms: {}, } as unknown as Table_Internal const featuresList: Array = Object.values(table._features) @@ -137,9 +143,7 @@ export function constructTable< ;(table.atoms as any)[key] = _reactivity.createReadonlyAtom( () => { const options = table.options - const externalAtoms = options.atoms as - | Partial>> - | undefined + const externalAtoms = options.atoms const externalAtom = externalAtoms?.[key] // Always touch the reactive owner so controlled state still has an // invalidation source when it is published after a framework commit. @@ -184,13 +188,6 @@ export function constructTable< ), ) - // pre-compute the init functions to make the other constructors faster - table._cellInstanceInitFns = [] - table._columnInstanceInitFns = [] - table._headerGroupInstanceInitFns = [] - table._headerInstanceInitFns = [] - table._rowInstanceInitFns = [] - for (let i = 0; i < featuresList.length; i++) { const feature = featuresList[i]! feature.initTableInstanceData?.(table) diff --git a/packages/table-core/src/core/table/coreTablesFeature.types.ts b/packages/table-core/src/core/table/coreTablesFeature.types.ts index 7dfaf87e80..715455cc67 100644 --- a/packages/table-core/src/core/table/coreTablesFeature.types.ts +++ b/packages/table-core/src/core/table/coreTablesFeature.types.ts @@ -164,9 +164,7 @@ export interface Table_CoreProperties< /** * Cache of the `initCellInstanceData` functions for features that define one. */ - _cellInstanceInitFns?: Array< - NonNullable - > + _cellInstanceInitFns: Array> /** * Prototype cache for Cell objects - shared by all cells in this table */ @@ -174,7 +172,7 @@ export interface Table_CoreProperties< /** * Cache of the `initColumnInstanceData` functions for features that define one. */ - _columnInstanceInitFns?: Array< + _columnInstanceInitFns: Array< NonNullable > /** @@ -188,13 +186,13 @@ export interface Table_CoreProperties< /** * Cache of the `initHeaderGroupInstanceData` functions for features that define one. */ - _headerGroupInstanceInitFns?: Array< + _headerGroupInstanceInitFns: Array< NonNullable > /** * Cache of the `initHeaderInstanceData` functions for features that define one. */ - _headerInstanceInitFns?: Array< + _headerInstanceInitFns: Array< NonNullable > /** @@ -216,7 +214,7 @@ export interface Table_CoreProperties< /** * Cache of the `initRowInstanceData` functions for features that define one. */ - _rowInstanceInitFns?: Array> + _rowInstanceInitFns: Array> /** * The readonly derived atoms for each `TableState` slice. Each derives from * its corresponding `baseAtom` plus, optionally, a per-slice external atom or