fix: remove options store from react/preact and refactor external state syncing#6244
Merged
KevinVandy merged 4 commits intoalphafrom May 7, 2026
Merged
fix: remove options store from react/preact and refactor external state syncing#6244KevinVandy merged 4 commits intoalphafrom
KevinVandy merged 4 commits intoalphafrom
Conversation
|
View your CI Pipeline Execution ↗ for commit 85cb98f
☁️ Nx Cloud last updated this comment at |
Contributor
📝 WalkthroughWalkthroughThis PR refactors table reactivity architecture by introducing conditional options store creation via a new ChangesReactivity Architecture Refactor
Sequence DiagramsequenceDiagram
participant App as Application
participant Table as Table Instance
participant RB as Reactivity Bindings
participant Store as Options Store
participant Atoms as Base Atoms
Note over App,Atoms: Table Construction with createOptionsStore: true
App->>Table: constructTable(options, features)
Table->>RB: Initialize TableReactivityBindings
RB->>Store: createOptionsStore enabled
Table->>Store: Create optionsStore Atom
Table->>Atoms: Create baseAtoms from state
Note over App,Atoms: Options Update Flow
App->>Table: setOptions(newOptions)
Table->>Store: Merge & update optionsStore
Table->>Atoms: table_syncExternalStateToBaseAtoms()
Atoms->>Atoms: Sync external state to baseAtoms
Note over App,Atoms: Reactive Reads
App->>Table: table.atoms[key].get()
Table->>Table: Check table.options.atoms[key]
Table->>Atoms: Fallback to baseAtoms[key]
Table-->>App: Return derived value
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
Refactors table option/state reactivity across adapters by making optionsStore optional (removed for React/Preact), introducing explicit Store-based reactivity bindings for vanilla usage, and updating external state syncing so options.state is mirrored into base atoms.
Changes:
- Make
table.optionsStoreconditional via a newcreateOptionsStoreflag onTableReactivityBindingsand adjustconstructTable/setOptionsaccordingly. - Add
store-reactivity-bindingsas a new@tanstack/table-coreentrypoint and update vanilla examples/tests to use it. - Update devtools panels to handle missing
optionsStoreand update docs/types to reflect the new model.
Reviewed changes
Copilot reviewed 69 out of 70 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates workspace lockfile for dependency changes (notably removes @preact/signals, adds Store-related entries). |
| packages/vue-table/src/reactivity.ts | Marks Vue bindings as createOptionsStore: true. |
| packages/table-devtools/src/components/StatePanel.tsx | Handles optional optionsStore and adjusts subscriptions for state/options display. |
| packages/table-devtools/src/components/OptionsPanel.tsx | Handles optional optionsStore and projects options for tree rendering. |
| packages/table-devtools/package.json | Adds @tanstack/table-core to devDependencies for local build/test. |
| packages/table-core/tsdown.config.ts | Adds store-reactivity-bindings entrypoint to build outputs. |
| packages/table-core/tests/unit/core/tableAtoms.test.ts | Switches tests to use storeReactivityBindings. |
| packages/table-core/tests/unit/core/table/stockFeaturesInitialState.test.ts | Switches tests to use storeReactivityBindings. |
| packages/table-core/tests/unit/core/table/constructTable.test.ts | Switches tests to use storeReactivityBindings. |
| packages/table-core/tests/unit/core/columns/constructColumn.test.ts | Switches tests to use storeReactivityBindings. |
| packages/table-core/tests/performance/features/column-grouping/columnGroupingFeature.test.ts | Switches perf test to use storeReactivityBindings. |
| packages/table-core/tests/implementation/features/row-selection/rowSelectionFeature.test.ts | Switches implementation test to use storeReactivityBindings. |
| packages/table-core/tests/implementation/features/row-pinning/rowPinningFeature.test.ts | Switches implementation tests to use storeReactivityBindings. |
| packages/table-core/tests/helpers/rowPinningHelpers.ts | Switches helper tables to use storeReactivityBindings. |
| packages/table-core/tests/helpers/generateTestTable.ts | Switches test table generators to use storeReactivityBindings. |
| packages/table-core/src/types/Table.ts | Removes unused ReadonlyAtom type import. |
| packages/table-core/src/store-reactivity-bindings.ts | New Store-based reactivity bindings entrypoint (vanilla/non-framework). |
| packages/table-core/src/reactivity.ts | Stops exporting the removed constructReactivityBindings. |
| packages/table-core/src/core/table/coreTablesFeature.utils.ts | Adds external state → base atom syncing; setOptions now supports missing optionsStore. |
| packages/table-core/src/core/table/coreTablesFeature.types.ts | Makes optionsStore optional and updates doc comment accordingly. |
| packages/table-core/src/core/table/constructTable.ts | Conditionally creates optionsStore and revises state atom derivation/external syncing. |
| packages/table-core/src/core/reactivity/coreReactivityFeature.types.ts | Adds createOptionsStore to reactivity bindings interface. |
| packages/table-core/src/core/reactivity/constructReactivityBindings.ts | Removes old Store-based bindings helper. |
| packages/table-core/package.json | Exports new ./store-reactivity-bindings subpath. |
| packages/svelte-table/src/reactivity.svelte.ts | Marks Svelte bindings as createOptionsStore: true and tweaks subscription untracking. |
| packages/solid-table/src/reactivity.ts | Marks Solid bindings as createOptionsStore: true. |
| packages/react-table/src/useTable.ts | Switches React adapter to custom bindings; removes optionsStore usage and changes option syncing behavior. |
| packages/react-table/src/reactivity.ts | New React-specific reactivity bindings (createOptionsStore: false). |
| packages/preact-table/tests/unit/signals.test.ts | Updates test description to reflect Store-based atoms. |
| packages/preact-table/src/useTable.ts | Switches Preact adapter to custom bindings; removes optionsStore usage and changes option syncing behavior. |
| packages/preact-table/src/reactivity.ts | Replaces signals-based approach with Store atoms; sets createOptionsStore: false. |
| packages/preact-table/package.json | Removes @preact/signals dependency. |
| packages/lit-table/src/TableController.ts | Switches Lit adapter to litReactivity() and updates options store subscription typing. |
| packages/lit-table/src/reactivity.ts | New Lit-specific reactivity bindings (createOptionsStore: true). |
| packages/angular-table/src/reactivity.ts | Marks Angular bindings as createOptionsStore: true. |
| packages/angular-table/src/injectTable.ts | Adjusts for optional optionsStore via non-null assertion under Angular bindings. |
| examples/vanilla/sorting/src/main.ts | Uses storeReactivityBindings() in vanilla example features. |
| examples/vanilla/pagination/src/main.ts | Uses storeReactivityBindings() in vanilla example features. |
| examples/vanilla/basic/src/main.ts | Uses storeReactivityBindings() in vanilla example features. |
| examples/react/basic-external-state/src/main.tsx | Removes noisy console logs (commented out). |
| docs/reference/variables/coreFeatures.md | Updates generated “Defined in” line references. |
| docs/reference/type-aliases/ExtractFeatureTypes.md | Excludes coreReativityFeature from feature-type extraction. |
| docs/reference/type-aliases/ExternalAtoms.md | Updates generated “Defined in” line references. |
| docs/reference/type-aliases/ExternalAtoms_All.md | Updates generated “Defined in” line references. |
| docs/reference/type-aliases/BaseAtoms.md | Updates generated “Defined in” line references. |
| docs/reference/type-aliases/BaseAtoms_All.md | Updates generated “Defined in” line references. |
| docs/reference/type-aliases/Atoms.md | Updates generated “Defined in” line references. |
| docs/reference/type-aliases/Atoms_All.md | Updates generated “Defined in” line references. |
| docs/reference/interfaces/TableReactivityFeatureConstructors.md | Removes generated docs for deleted interface. |
| docs/reference/interfaces/TableOptions_Table.md | Updates generated docs types/readonly markers and line references. |
| docs/reference/interfaces/TableOptions_Core.md | Updates generated docs types/readonly markers and line references. |
| docs/reference/interfaces/TableMeta.md | Updates generated “Defined in” line references. |
| docs/reference/interfaces/TableFeatures.md | Adds/updates coreReativityFeature docs and line references. |
| docs/reference/interfaces/Table_Table.md | Updates optionsStore docs to optional and refreshes line references. |
| docs/reference/interfaces/Table_CoreProperties.md | Updates optionsStore docs to optional and refreshes line references. |
| docs/reference/interfaces/CoreFeatures.md | Adds/updates coreReativityFeature docs and line references. |
| docs/reference/index.md | Removes references to deleted reactivity feature docs. |
| docs/reference/functions/tableOptions.md | Updates generated overload signatures and line references. |
| docs/reference/functions/getInitialTableState.md | Updates generated “Defined in” line reference. |
| docs/reference/functions/constructTable.md | Updates generated “Defined in” line reference. |
| docs/reference/functions/constructReactivityFeature.md | Removes generated docs for deleted function. |
| docs/framework/react/reference/index/type-aliases/ReactTable.md | Updates ReactTable state type docs and line references. |
| docs/framework/react/reference/index/interfaces/AppHeaderPropsWithSelector.md | Updates generated type to exclude coreReativityFeature. |
| docs/framework/react/reference/index/interfaces/AppHeaderPropsWithoutSelector.md | Updates generated type to exclude coreReativityFeature. |
| docs/framework/react/reference/index/interfaces/AppCellPropsWithSelector.md | Updates generated type to exclude coreReativityFeature. |
| docs/framework/react/reference/index/interfaces/AppCellPropsWithoutSelector.md | Updates generated type to exclude coreReativityFeature. |
| docs/framework/react/reference/index/functions/useTable.md | Updates generated “Defined in” line reference. |
| docs/framework/angular/reference/type-aliases/AngularTable.md | Updates AngularTable docs around computed API and line references. |
| docs/framework/angular/reference/interfaces/AngularTableComputed.md | Updates generated “Defined in” line references. |
| docs/framework/angular/reference/functions/injectTable.md | Updates generated “Defined in” line reference. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
pnpm-lock.yaml:7147
pnpm-lock.yamllists@tanstack/storeas a direct dependency forpackages/preact-table, butpackages/preact-table/package.jsondoes not declare it. Please either add the dependency topackage.json(if intentionally required) or regenerate the lockfile so the importer snapshot matches the workspace manifests.
pnpm-lock.yaml:7185pnpm-lock.yamllists@tanstack/storeas a direct dependency forpackages/react-table, butpackages/react-table/package.jsondoes not declare it. Please reconcile this by updating the manifest (if needed) or regenerating the lockfile.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+142
to
+146
| // sync options on every render | ||
| table.setOptions((prev) => ({ | ||
| ...prev, | ||
| ...tableOptions, | ||
| })) |
Comment on lines
152
to
156
| return useMemo( | ||
| () => ({ | ||
| ...table, | ||
| options, | ||
| options: tableOptions, | ||
| state, |
Comment on lines
+113
to
+117
| // sync options on every render | ||
| table.setOptions((prev) => ({ | ||
| ...prev, | ||
| ...tableOptions, | ||
| })) |
Comment on lines
121
to
125
| return useMemo( | ||
| () => ({ | ||
| ...table, | ||
| options, | ||
| options: tableOptions, | ||
| state, |
Comment on lines
+170
to
174
| * Writable atom for table options. Only created when `createOptionsStore` is true | ||
| * on `coreReativityFeature` (e.g. Lit, Vue, Solid). React/Preact use `createOptionsStore: false`: | ||
| * options are held as plain data with a lightweight revision + subscribe source; there is | ||
| * no writable atom backing the full options object. Use {@link getTableOptionsStore} to subscribe. | ||
| */ |
Comment on lines
+53
to
+57
| return table.optionsStore!.get() | ||
| }, | ||
| set(value) { | ||
| table.optionsStore!.set(() => value) // or your real update shape | ||
| }, |
Comment on lines
+28
to
+32
| projectOptionsForTree(state), | ||
| ) | ||
| : useTableStore(tableInstance.store, () => | ||
| projectOptionsForTree(tableInstance.options as unknown), | ||
| ) |
Comment on lines
+32
to
+36
| const tableOptions = tableInstance | ||
| ? tableInstance.optionsStore | ||
| ? useSelector(tableInstance.optionsStore, (opts) => opts) | ||
| : useTableStore(tableInstance.store, () => tableInstance.options) | ||
| : undefined |
Comment on lines
+25
to
+29
| // // TOTO - re-explore preact signals for reactivity | ||
| // import { batch, computed, signal, untracked } from '@preact/signals' | ||
| // import type { | ||
| // TableAtomOptions, | ||
| // TableReactivityBindings, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/angular-table/src/injectTable.ts (1)
176-185:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse optional chaining instead of non-null assertion for
optionsStoreIn the
computedcallback (lines 176-185), replace the non-null assertion with optional chaining:Proposed fix
- table.optionsStore!.get() + table.optionsStore?.get()While
createOptionsStore: trueis currently set inangularReactivity, optional chaining is a more defensive pattern. It returnsundefinedfor the side-effect call ifoptionsStoreis uninitialized, preventing potential runtime errors from future misconfigurations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/angular-table/src/injectTable.ts` around lines 176 - 185, Replace the non-null assertion on table.optionsStore in the computed callback with optional chaining: locate the Object.defineProperty call setting 'value' where computed's getter calls table.store.get() and table.optionsStore!.get(), and change the latter to table.optionsStore?.get() so the side-effect is skipped safely if optionsStore is undefined while leaving the rest of the computed behavior intact.packages/table-devtools/src/components/OptionsPanel.tsx (1)
39-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRedundant double invocation of
tableState?.()
tableState?.()is called twice insidegetState()— the first call discards its return value and serves no additional purpose in Solid's reactive tracking (signal reads are deduplicated within a computation). This adds noise and may mislead future maintainers into thinking the first call has a side effect.🛠️ Proposed fix
const getState = (): unknown => { - tableState?.() return tableState?.() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-devtools/src/components/OptionsPanel.tsx` around lines 39 - 42, The getState function calls tableState?.() twice; remove the redundant first invocation and simply return the signal read once to avoid a discarded call and confusion. Update the getState implementation (referencing getState and tableState) so it either directly returns tableState?.() or captures the result in a local variable and returns that single value, ensuring no extra signal read is performed.docs/reference/functions/tableOptions.md (1)
92-204:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisambiguate the repeated
## Call Signatureheadings.These repeated H2s are what markdownlint is flagging with MD024, so this page will keep failing docs lint until each overload section gets a unique heading/title. If this file is generated, the template should be fixed instead of patching the output by hand.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/functions/tableOptions.md` around lines 92 - 204, The docs page contains multiple identical H2 headings "## Call Signature" for the function tableOptions which triggers markdownlint MD024; update the generator/template (or the static file if necessary) so each overload section has a unique H2 — e.g. change the headings to include the overload signature or an index like "## Call Signature — tableOptions<TFeatures, TData> (overload 1)" for the entries defined at helpers/tableOptions.ts:38, :43, :50, :57 so each overload block is distinct and MD024 is resolved.
♻️ Duplicate comments (4)
docs/framework/react/reference/index/interfaces/AppHeaderPropsWithSelector.md (1)
50-50:⚠️ Potential issue | 🔴 CriticalSame typo: "coreReativityFeature" should be "coreReactivityFeature".
This documentation reflects the same misspelling flagged in
ExtractFeatureTypes.md. The source TypeScript code needs to be corrected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/react/reference/index/interfaces/AppHeaderPropsWithSelector.md` at line 50, Replace the misspelled key "coreReativityFeature" with the correct "coreReactivityFeature" in the type mappings so the feature-extraction types resolve correctly; update every occurrence in the ExtractFeatureTypes-related types (references in UnionToIntersection/FeatureConstructorOptions/TFeatures contexts and any docs derived from them) to use "coreReactivityFeature" so the conditional type K extends "coreReactivityFeature" ? never : ... matches the actual feature key.docs/framework/react/reference/index/interfaces/AppCellPropsWithSelector.md (1)
58-58:⚠️ Potential issue | 🔴 CriticalSame typo: "coreReativityFeature" should be "coreReactivityFeature".
This documentation reflects the same misspelling flagged in
ExtractFeatureTypes.md. The source TypeScript code needs to be corrected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/react/reference/index/interfaces/AppCellPropsWithSelector.md` at line 58, The typedef in the documentation and source TypeScript contains a typo: rename the symbol "coreReativityFeature" to "coreReactivityFeature" wherever it is defined/used so the conditional type in the compound type (involving UnionToIntersection, Cell_Cell, Cell_ColumnGrouping, Cell_Plugins and the mapped feature extraction logic) correctly matches the core reactivity feature; update the source definition that feeds ExtractFeatureTypes and AppCellPropsWithSelector (the mapped key in the feature extraction type) to the corrected "coreReactivityFeature" string so docs regenerate with the correct spelling.docs/framework/react/reference/index/interfaces/AppCellPropsWithoutSelector.md (1)
54-54:⚠️ Potential issue | 🔴 CriticalSame typo: "coreReativityFeature" should be "coreReactivityFeature".
This documentation reflects the same misspelling flagged in
ExtractFeatureTypes.md. The source TypeScript code needs to be corrected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/react/reference/index/interfaces/AppCellPropsWithoutSelector.md` at line 54, The docs show a typo: rename the string/key "coreReativityFeature" to "coreReactivityFeature" in the TypeScript feature-extraction types; update every occurrence in the type mapping logic (e.g., the UnionToIntersection mapping used alongside Cell_Cell, Cell_ColumnGrouping, Cell_Plugins, TCellComponents and the ExtractFeatureTypes utilities) so the conditional K extends "coreReactivityFeature" is correct and the feature lookup resolves properly.docs/framework/react/reference/index/interfaces/AppHeaderPropsWithoutSelector.md (1)
46-46:⚠️ Potential issue | 🔴 CriticalSame typo: "coreReativityFeature" should be "coreReactivityFeature".
This documentation reflects the same misspelling flagged in
ExtractFeatureTypes.md. The source TypeScript code needs to be corrected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/react/reference/index/interfaces/AppHeaderPropsWithoutSelector.md` at line 46, The docs show a typo: replace the misspelled key "coreReativityFeature" with "coreReactivityFeature" in the source TypeScript where feature keys/types are defined (the code that constructs the mapped type over TFeatures used by ExtractFeatureTypes / AppHeaderPropsWithoutSelector), so update the string literal/key used in the conditional mapping (the spot that currently reads K extends "coreReativityFeature" ? never : ...) to K extends "coreReactivityFeature" ? never : ... ensuring the change is applied where TFeatures, ExtractFeatureTypes, or the Header-related mapped types are declared.
🧹 Nitpick comments (2)
packages/preact-table/tests/unit/signals.test.ts (1)
5-19: ⚡ Quick winTest only exercises synchronous
get()— consider adding asubscribepathThe rewrite changed both the sync and async (subscribe/unsubscribe) contracts of
createWritableAtom/createReadonlyAtom. The existing test only validatesget()afterset(). A short subscription test would confirm the@tanstack/preact-storewiring notifies observers correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/preact-table/tests/unit/signals.test.ts` around lines 5 - 19, The test only asserts synchronous get() behavior and misses verifying subscriptions; update the unit test in signals.test.ts to also exercise the subscription path by subscribing to the writable atom and asserting the subscriber is called with updated values after count.set(...), then unsubscribe and ensure no further notifications; specifically use createWritableAtom and createReadonlyAtom from preactReactivity(), call subscribe on the writable atom (and optionally readonly) to capture notifications, assert the callback receives the expected sequence (1 then 2 for count, and 2 then 4 for doubled), and finally call the returned unsubscribe function to verify notifications stop.packages/preact-table/src/reactivity.ts (1)
25-87: ⚡ Quick winRemove the commented-out
@preact/signalsblock — dead code with a mistypedTOTOmarker63 lines of commented-out code (plus
// TOTO - re-explore preact signals for reactivitywith a misspelling) adds noise and risks being treated as authoritative. If there's intent to revisit signals, track it in an issue rather than leaving it in source.🗑️ Proposed change
-// // 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<T>( -// ... -// }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/preact-table/src/reactivity.ts` around lines 25 - 87, Remove the entire commented-out `@preact/signals` block (including the "TOTO - re-explore preact signals for reactivity" line) to eliminate dead code and the misspelled TODO; specifically delete the observerToCallback, signalToReadonlyAtom, signalToWritableAtom functions and the preactReactivity implementation that are commented out, and if the intent to revisit signals should be preserved, create an issue or add a proper TODO (correctly spelled) elsewhere instead of leaving the large commented block in the source.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/interfaces/TableFeatures.md`:
- Around line 170-181: The property name coreReativityFeature is misspelled;
rename the symbol to coreReactivityFeature across source and docs and update all
references. Specifically, change the identifier in the CoreFeatures interface
and its declaration in coreFeatures.ts (and any usage in main.ts mentioned in
the companion comment) to coreReactivityFeature, then update the generated docs
entry in TableFeatures.md (and any other docs) to use the new
coreReactivityFeature name so all type definitions, references, and links remain
consistent.
In `@examples/vanilla/pagination/src/main.ts`:
- Line 20: Rename the misspelled feature key coreReativityFeature to
coreReactivityFeature everywhere it is exported/used so the public API is
correct; update the instance in this file where tableFeatures is called (replace
coreReativityFeature: storeReactivityBindings() with coreReactivityFeature:
storeReactivityBindings()) and search/replace occurrences across examples, docs,
and the store-reactivity-bindings module to keep exports, imports, and tests
consistent (ensure any named exports or consumers referencing
coreReativityFeature are updated to coreReactivityFeature).
In `@packages/lit-table/src/TableController.ts`:
- Around line 218-222: The code is using a non-null assertion on
this._table.optionsStore when subscribing; replace that with a runtime guard:
check if this._table.optionsStore is defined before calling subscribe (and only
assign this._optionsSubscription inside that branch), remove the eslint-disable
comment, and ensure any existing _optionsSubscription is properly
unsubscribed/cleared when optionsStore is undefined so you don't leak
subscriptions; keep the existing behavior of incrementing this._notifier and
calling this.host.requestUpdate() from the subscription callback.
In `@packages/table-core/src/core/table/constructTable.ts`:
- Around line 42-61: The constructor currently uses a raw spread to create
mergedOptions which bypasses the library's merge logic; replace the spread with
a call to the merge helper (the same function used by setOptions, e.g.
table_mergeOptions or mergeOptions) so the initial options respect
reactive/proxy-backed shapes. Concretely, compute mergedOptions =
mergeOptions(defaultOptions, tableOptions) (or call
table.mergeOptions(defaultOptions, tableOptions)) and then use that
mergedOptions when creating optionsStore or assigning table.options so adapters
get the correct shape from the start.
In `@packages/table-core/src/core/table/coreTablesFeature.types.ts`:
- Around line 170-175: The JSDoc references a non-existent function
getTableOptionsStore which creates a broken hover link; either implement
getTableOptionsStore or update the comment to reference the actual API (e.g.,
the readonly optionsStore field and createOptionsStore flag) — locate the
comment around optionsStore in coreTablesFeature.types (symbols: optionsStore,
createOptionsStore, coreReativityFeature) and remove or replace {`@link`
getTableOptionsStore} with a valid reference such as "optionsStore" or explain
that React/Preact consumers should use the lightweight revision/subscribe API
instead.
In `@packages/table-core/src/core/table/coreTablesFeature.utils.ts`:
- Around line 67-75: The current branch updates the external optionsStore which
can notify subscribers before base atoms are synchronized, causing adapters to
see new options with stale table state; to fix this, when table.optionsStore is
present compute mergedOptions via functionalUpdate and table_mergeOptions, then
call table_syncExternalStateToBaseAtoms(table) before publishing mergedOptions
to optionsStore (or perform the set with a callback that first syncs base atoms
and then returns mergedOptions), otherwise keep the existing fallback that
assigns table.options; reference optionsStore, functionalUpdate,
table_mergeOptions, table_syncExternalStateToBaseAtoms, and table.options to
locate and adjust the update order.
In `@packages/table-devtools/src/components/OptionsPanel.tsx`:
- Around line 25-33: The fallback branch currently subscribes to
tableInstance.store but reads the non-reactive tableInstance.options directly
(in the useTableStore selector passed to projectOptionsForTree), causing stale
UI when createOptionsStore is false; update the selector used in the
useTableStore fallback to read a reactive marker together with the options so
the selector re-runs when options change — e.g., read a lightweight revision or
accessor exposed by the table (such as an optionsRevision or getOptions
accessor) inside the selector and then return
projectOptionsForTree(tableInstance.options) (or
projectOptionsForTree(tableInstance.getOptions())) so the OptionsPanel updates
whenever setOptions increments that revision or the accessor reflects new
options; ensure references to tableInstance, useTableStore,
projectOptionsForTree, tableInstance.options (or tableInstance.getOptions) and
the new revision/accessor are used in the selector.
---
Outside diff comments:
In `@docs/reference/functions/tableOptions.md`:
- Around line 92-204: The docs page contains multiple identical H2 headings "##
Call Signature" for the function tableOptions which triggers markdownlint MD024;
update the generator/template (or the static file if necessary) so each overload
section has a unique H2 — e.g. change the headings to include the overload
signature or an index like "## Call Signature — tableOptions<TFeatures, TData>
(overload 1)" for the entries defined at helpers/tableOptions.ts:38, :43, :50,
:57 so each overload block is distinct and MD024 is resolved.
In `@packages/angular-table/src/injectTable.ts`:
- Around line 176-185: Replace the non-null assertion on table.optionsStore in
the computed callback with optional chaining: locate the Object.defineProperty
call setting 'value' where computed's getter calls table.store.get() and
table.optionsStore!.get(), and change the latter to table.optionsStore?.get() so
the side-effect is skipped safely if optionsStore is undefined while leaving the
rest of the computed behavior intact.
In `@packages/table-devtools/src/components/OptionsPanel.tsx`:
- Around line 39-42: The getState function calls tableState?.() twice; remove
the redundant first invocation and simply return the signal read once to avoid a
discarded call and confusion. Update the getState implementation (referencing
getState and tableState) so it either directly returns tableState?.() or
captures the result in a local variable and returns that single value, ensuring
no extra signal read is performed.
---
Duplicate comments:
In
`@docs/framework/react/reference/index/interfaces/AppCellPropsWithoutSelector.md`:
- Line 54: The docs show a typo: rename the string/key "coreReativityFeature" to
"coreReactivityFeature" in the TypeScript feature-extraction types; update every
occurrence in the type mapping logic (e.g., the UnionToIntersection mapping used
alongside Cell_Cell, Cell_ColumnGrouping, Cell_Plugins, TCellComponents and the
ExtractFeatureTypes utilities) so the conditional K extends
"coreReactivityFeature" is correct and the feature lookup resolves properly.
In `@docs/framework/react/reference/index/interfaces/AppCellPropsWithSelector.md`:
- Line 58: The typedef in the documentation and source TypeScript contains a
typo: rename the symbol "coreReativityFeature" to "coreReactivityFeature"
wherever it is defined/used so the conditional type in the compound type
(involving UnionToIntersection, Cell_Cell, Cell_ColumnGrouping, Cell_Plugins and
the mapped feature extraction logic) correctly matches the core reactivity
feature; update the source definition that feeds ExtractFeatureTypes and
AppCellPropsWithSelector (the mapped key in the feature extraction type) to the
corrected "coreReactivityFeature" string so docs regenerate with the correct
spelling.
In
`@docs/framework/react/reference/index/interfaces/AppHeaderPropsWithoutSelector.md`:
- Line 46: The docs show a typo: replace the misspelled key
"coreReativityFeature" with "coreReactivityFeature" in the source TypeScript
where feature keys/types are defined (the code that constructs the mapped type
over TFeatures used by ExtractFeatureTypes / AppHeaderPropsWithoutSelector), so
update the string literal/key used in the conditional mapping (the spot that
currently reads K extends "coreReativityFeature" ? never : ...) to K extends
"coreReactivityFeature" ? never : ... ensuring the change is applied where
TFeatures, ExtractFeatureTypes, or the Header-related mapped types are declared.
In
`@docs/framework/react/reference/index/interfaces/AppHeaderPropsWithSelector.md`:
- Line 50: Replace the misspelled key "coreReativityFeature" with the correct
"coreReactivityFeature" in the type mappings so the feature-extraction types
resolve correctly; update every occurrence in the ExtractFeatureTypes-related
types (references in UnionToIntersection/FeatureConstructorOptions/TFeatures
contexts and any docs derived from them) to use "coreReactivityFeature" so the
conditional type K extends "coreReactivityFeature" ? never : ... matches the
actual feature key.
---
Nitpick comments:
In `@packages/preact-table/src/reactivity.ts`:
- Around line 25-87: Remove the entire commented-out `@preact/signals` block
(including the "TOTO - re-explore preact signals for reactivity" line) to
eliminate dead code and the misspelled TODO; specifically delete the
observerToCallback, signalToReadonlyAtom, signalToWritableAtom functions and the
preactReactivity implementation that are commented out, and if the intent to
revisit signals should be preserved, create an issue or add a proper TODO
(correctly spelled) elsewhere instead of leaving the large commented block in
the source.
In `@packages/preact-table/tests/unit/signals.test.ts`:
- Around line 5-19: The test only asserts synchronous get() behavior and misses
verifying subscriptions; update the unit test in signals.test.ts to also
exercise the subscription path by subscribing to the writable atom and asserting
the subscriber is called with updated values after count.set(...), then
unsubscribe and ensure no further notifications; specifically use
createWritableAtom and createReadonlyAtom from preactReactivity(), call
subscribe on the writable atom (and optionally readonly) to capture
notifications, assert the callback receives the expected sequence (1 then 2 for
count, and 2 then 4 for doubled), and finally call the returned unsubscribe
function to verify notifications stop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee71f98b-a672-4112-a636-38a8d04680ec
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (69)
docs/framework/angular/reference/functions/injectTable.mddocs/framework/angular/reference/interfaces/AngularTableComputed.mddocs/framework/angular/reference/type-aliases/AngularTable.mddocs/framework/react/reference/index/functions/useTable.mddocs/framework/react/reference/index/interfaces/AppCellPropsWithSelector.mddocs/framework/react/reference/index/interfaces/AppCellPropsWithoutSelector.mddocs/framework/react/reference/index/interfaces/AppHeaderPropsWithSelector.mddocs/framework/react/reference/index/interfaces/AppHeaderPropsWithoutSelector.mddocs/framework/react/reference/index/type-aliases/ReactTable.mddocs/reference/functions/constructReactivityFeature.mddocs/reference/functions/constructTable.mddocs/reference/functions/getInitialTableState.mddocs/reference/functions/tableOptions.mddocs/reference/index.mddocs/reference/interfaces/CoreFeatures.mddocs/reference/interfaces/TableFeatures.mddocs/reference/interfaces/TableMeta.mddocs/reference/interfaces/TableOptions_Core.mddocs/reference/interfaces/TableOptions_Table.mddocs/reference/interfaces/TableReactivityFeatureConstructors.mddocs/reference/interfaces/Table_CoreProperties.mddocs/reference/interfaces/Table_Table.mddocs/reference/type-aliases/Atoms.mddocs/reference/type-aliases/Atoms_All.mddocs/reference/type-aliases/BaseAtoms.mddocs/reference/type-aliases/BaseAtoms_All.mddocs/reference/type-aliases/ExternalAtoms.mddocs/reference/type-aliases/ExternalAtoms_All.mddocs/reference/type-aliases/ExtractFeatureTypes.mddocs/reference/variables/coreFeatures.mdexamples/react/basic-external-state/src/main.tsxexamples/vanilla/basic/src/main.tsexamples/vanilla/pagination/src/main.tsexamples/vanilla/sorting/src/main.tspackages/angular-table/src/injectTable.tspackages/angular-table/src/reactivity.tspackages/lit-table/src/TableController.tspackages/lit-table/src/reactivity.tspackages/preact-table/package.jsonpackages/preact-table/src/reactivity.tspackages/preact-table/src/useTable.tspackages/preact-table/tests/unit/signals.test.tspackages/react-table/src/reactivity.tspackages/react-table/src/useTable.tspackages/solid-table/src/reactivity.tspackages/svelte-table/src/reactivity.svelte.tspackages/table-core/package.jsonpackages/table-core/src/core/reactivity/constructReactivityBindings.tspackages/table-core/src/core/reactivity/coreReactivityFeature.types.tspackages/table-core/src/core/table/constructTable.tspackages/table-core/src/core/table/coreTablesFeature.types.tspackages/table-core/src/core/table/coreTablesFeature.utils.tspackages/table-core/src/reactivity.tspackages/table-core/src/store-reactivity-bindings.tspackages/table-core/src/types/Table.tspackages/table-core/tests/helpers/generateTestTable.tspackages/table-core/tests/helpers/rowPinningHelpers.tspackages/table-core/tests/implementation/features/row-pinning/rowPinningFeature.test.tspackages/table-core/tests/implementation/features/row-selection/rowSelectionFeature.test.tspackages/table-core/tests/performance/features/column-grouping/columnGroupingFeature.test.tspackages/table-core/tests/unit/core/columns/constructColumn.test.tspackages/table-core/tests/unit/core/table/constructTable.test.tspackages/table-core/tests/unit/core/table/stockFeaturesInitialState.test.tspackages/table-core/tests/unit/core/tableAtoms.test.tspackages/table-core/tsdown.config.tspackages/table-devtools/package.jsonpackages/table-devtools/src/components/OptionsPanel.tsxpackages/table-devtools/src/components/StatePanel.tsxpackages/vue-table/src/reactivity.ts
💤 Files with no reviewable changes (6)
- docs/reference/index.md
- packages/preact-table/package.json
- docs/reference/interfaces/TableReactivityFeatureConstructors.md
- packages/table-core/src/core/reactivity/constructReactivityBindings.ts
- packages/table-core/src/reactivity.ts
- docs/reference/functions/constructReactivityFeature.md
Comment on lines
+170
to
+181
| ### coreReativityFeature? | ||
|
|
||
| ```ts | ||
| optional coreReativityFeature: TableReactivityBindings; | ||
| ``` | ||
|
|
||
| Defined in: [core/coreFeatures.ts:10](https://github.com/TanStack/table/blob/main/packages/table-core/src/core/coreFeatures.ts#L10) | ||
|
|
||
| #### Inherited from | ||
|
|
||
| [`CoreFeatures`](CoreFeatures.md).[`coreReativityFeature`](CoreFeatures.md#corereativityfeature) | ||
|
|
Contributor
There was a problem hiding this comment.
coreReativityFeature documentation reflects the same API typo — fix together with source.
The documented property name is misspelled (Reativity → Reactivity). This doc entry will need to be updated at the same time the source symbol is renamed (see companion comment in main.ts).
🧰 Tools
🪛 LanguageTool
[grammar] ~170-~170: Ensure spelling is correct
Context: ...atures.md#coreheadersfeature) *** ### coreReativityFeature? ts optional coreReativityFeature: TableReactivityBindings; Defined in: [core/coreFeatures.ts:10](ht...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/reference/interfaces/TableFeatures.md` around lines 170 - 181, The
property name coreReativityFeature is misspelled; rename the symbol to
coreReactivityFeature across source and docs and update all references.
Specifically, change the identifier in the CoreFeatures interface and its
declaration in coreFeatures.ts (and any usage in main.ts mentioned in the
companion comment) to coreReactivityFeature, then update the generated docs
entry in TableFeatures.md (and any other docs) to use the new
coreReactivityFeature name so all type definitions, references, and links remain
consistent.
|
|
||
| const _features = tableFeatures({ | ||
| rowPaginationFeature, | ||
| coreReativityFeature: storeReactivityBindings(), |
Contributor
There was a problem hiding this comment.
coreReativityFeature is a misspelling of coreReactivityFeature — public API typo worth fixing on alpha
The property key is missing the 'c' in "Reactivity". Because this is a named key consumers must pass to tableFeatures({...}), a rename later will be a breaking change. The typo is pervasive across the codebase (docs, examples, store-reactivity-bindings), so it's cheapest to fix now while still in alpha.
✏️ Proposed fix (this file)
- coreReativityFeature: storeReactivityBindings(),
+ coreReactivityFeature: storeReactivityBindings(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| coreReativityFeature: storeReactivityBindings(), | |
| coreReactivityFeature: storeReactivityBindings(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/vanilla/pagination/src/main.ts` at line 20, Rename the misspelled
feature key coreReativityFeature to coreReactivityFeature everywhere it is
exported/used so the public API is correct; update the instance in this file
where tableFeatures is called (replace coreReativityFeature:
storeReactivityBindings() with coreReactivityFeature: storeReactivityBindings())
and search/replace occurrences across examples, docs, and the
store-reactivity-bindings module to keep exports, imports, and tests consistent
(ensure any named exports or consumers referencing coreReativityFeature are
updated to coreReactivityFeature).
Comment on lines
+218
to
222
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion | ||
| this._optionsSubscription = this._table.optionsStore!.subscribe(() => { | ||
| this._notifier++ | ||
| this.host.requestUpdate() | ||
| }) |
Contributor
There was a problem hiding this comment.
Replace non-null assertion on optional optionsStore with a runtime guard
optionsStore is now typed as Atom<...> | undefined. The ! non-null assertion at line 219 sidesteps type safety rather than expressing the intent. Additionally, the @typescript-eslint/no-unnecessary-type-assertion suppression is logically incorrect here — since optionsStore is optional, the assertion is necessary, not unnecessary. If the linter fires on this line, it suggests the type visible to the checker is already non-null (a potential type inconsistency), and the comment suppresses a diagnostic that is actively pointing at a problem.
If litReactivity() is ever swapped for a custom binding with createOptionsStore: false, this will throw at runtime with no guard.
🛡️ Proposed fix
- // eslint-disable-next-line `@typescript-eslint/no-unnecessary-type-assertion`
- this._optionsSubscription = this._table.optionsStore!.subscribe(() => {
- this._notifier++
- this.host.requestUpdate()
- })
+ if (this._table.optionsStore) {
+ this._optionsSubscription = this._table.optionsStore.subscribe(() => {
+ this._notifier++
+ this.host.requestUpdate()
+ })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion | |
| this._optionsSubscription = this._table.optionsStore!.subscribe(() => { | |
| this._notifier++ | |
| this.host.requestUpdate() | |
| }) | |
| if (this._table.optionsStore) { | |
| this._optionsSubscription = this._table.optionsStore.subscribe(() => { | |
| this._notifier++ | |
| this.host.requestUpdate() | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lit-table/src/TableController.ts` around lines 218 - 222, The code
is using a non-null assertion on this._table.optionsStore when subscribing;
replace that with a runtime guard: check if this._table.optionsStore is defined
before calling subscribe (and only assign this._optionsSubscription inside that
branch), remove the eslint-disable comment, and ensure any existing
_optionsSubscription is properly unsubscribed/cleared when optionsStore is
undefined so you don't leak subscriptions; keep the existing behavior of
incrementing this._notifier and calling this.host.requestUpdate() from the
subscription callback.
Comment on lines
+42
to
+61
| const mergedOptions = { ...defaultOptions, ...tableOptions } | ||
|
|
||
| if (_reactivity.createOptionsStore) { | ||
| // @ts-ignore - direct set | ||
| table.optionsStore = _reactivity.createWritableAtom< | ||
| TableOptions<TFeatures, TData> | ||
| >(mergedOptions, { debugName: 'table/optionsStore' }) | ||
| Object.defineProperty(table, 'options', { | ||
| configurable: true, | ||
| enumerable: true, | ||
| get() { | ||
| return table.optionsStore!.get() | ||
| }, | ||
| set(value) { | ||
| table.optionsStore!.set(() => value) // or your real update shape | ||
| }, | ||
| }) | ||
| } else { | ||
| table.options = mergedOptions | ||
| } |
Contributor
There was a problem hiding this comment.
Honor mergeOptions during the initial options merge.
table_setOptions() still uses table_mergeOptions(), but the constructor now snapshots defaultOptions and tableOptions with a raw spread. Any adapter relying on mergeOptions to preserve reactive/proxy-backed option objects will start from the wrong shape until the first setOptions() call.
Suggested fix
- const mergedOptions = { ...defaultOptions, ...tableOptions }
+ const mergeOptions = tableOptions.mergeOptions ?? defaultOptions.mergeOptions
+ const mergedOptions = mergeOptions
+ ? mergeOptions(defaultOptions, tableOptions)
+ : { ...defaultOptions, ...tableOptions }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const mergedOptions = { ...defaultOptions, ...tableOptions } | |
| if (_reactivity.createOptionsStore) { | |
| // @ts-ignore - direct set | |
| table.optionsStore = _reactivity.createWritableAtom< | |
| TableOptions<TFeatures, TData> | |
| >(mergedOptions, { debugName: 'table/optionsStore' }) | |
| Object.defineProperty(table, 'options', { | |
| configurable: true, | |
| enumerable: true, | |
| get() { | |
| return table.optionsStore!.get() | |
| }, | |
| set(value) { | |
| table.optionsStore!.set(() => value) // or your real update shape | |
| }, | |
| }) | |
| } else { | |
| table.options = mergedOptions | |
| } | |
| const mergedOptions = (() => { | |
| const mergeOptions = tableOptions.mergeOptions ?? defaultOptions.mergeOptions | |
| return mergeOptions | |
| ? mergeOptions(defaultOptions, tableOptions) | |
| : { ...defaultOptions, ...tableOptions } | |
| })() | |
| if (_reactivity.createOptionsStore) { | |
| // `@ts-ignore` - direct set | |
| table.optionsStore = _reactivity.createWritableAtom< | |
| TableOptions<TFeatures, TData> | |
| >(mergedOptions, { debugName: 'table/optionsStore' }) | |
| Object.defineProperty(table, 'options', { | |
| configurable: true, | |
| enumerable: true, | |
| get() { | |
| return table.optionsStore!.get() | |
| }, | |
| set(value) { | |
| table.optionsStore!.set(() => value) // or your real update shape | |
| }, | |
| }) | |
| } else { | |
| table.options = mergedOptions | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/table-core/src/core/table/constructTable.ts` around lines 42 - 61,
The constructor currently uses a raw spread to create mergedOptions which
bypasses the library's merge logic; replace the spread with a call to the merge
helper (the same function used by setOptions, e.g. table_mergeOptions or
mergeOptions) so the initial options respect reactive/proxy-backed shapes.
Concretely, compute mergedOptions = mergeOptions(defaultOptions, tableOptions)
(or call table.mergeOptions(defaultOptions, tableOptions)) and then use that
mergedOptions when creating optionsStore or assigning table.options so adapters
get the correct shape from the start.
Comment on lines
+170
to
+175
| * Writable atom for table options. Only created when `createOptionsStore` is true | ||
| * on `coreReativityFeature` (e.g. Lit, Vue, Solid). React/Preact use `createOptionsStore: false`: | ||
| * options are held as plain data with a lightweight revision + subscribe source; there is | ||
| * no writable atom backing the full options object. Use {@link getTableOptionsStore} to subscribe. | ||
| */ | ||
| readonly optionsStore: Atom<TableOptions<TFeatures, TData>> | ||
| readonly optionsStore?: Atom<TableOptions<TFeatures, TData>> | undefined |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify getTableOptionsStore is defined and exported in table-core
rg -rn --type=ts "getTableOptionsStore" --include="*.ts" -C2Repository: TanStack/table
Length of output: 500
🏁 Script executed:
#!/bin/bash
# Search for getTableOptionsStore definition/export with correct rg syntax
rg "getTableOptionsStore" -t ts -C 2Repository: TanStack/table
Length of output: 745
🏁 Script executed:
#!/bin/bash
# Search for any variation of getTableOptions or TableOptionsStore
rg "getTableOptions|TableOptionsStore" -t ts -t jsRepository: TanStack/table
Length of output: 222
Remove or implement the undocumented getTableOptionsStore reference
The JSDoc at line 173 references {@link getTableOptionsStore}, but this function does not exist in the codebase. This will render as a broken link in IDE hover documentation. Either implement the function or update the comment to point to a valid API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/table-core/src/core/table/coreTablesFeature.types.ts` around lines
170 - 175, The JSDoc references a non-existent function getTableOptionsStore
which creates a broken hover link; either implement getTableOptionsStore or
update the comment to reference the actual API (e.g., the readonly optionsStore
field and createOptionsStore flag) — locate the comment around optionsStore in
coreTablesFeature.types (symbols: optionsStore, createOptionsStore,
coreReativityFeature) and remove or replace {`@link` getTableOptionsStore} with a
valid reference such as "optionsStore" or explain that React/Preact consumers
should use the lightweight revision/subscribe API instead.
Comment on lines
67
to
75
| const newOptions = functionalUpdate(updater, table.options) | ||
| const mergedOptions = table_mergeOptions(table, newOptions) | ||
| table.optionsStore.set(() => mergedOptions) | ||
| if (table.optionsStore) { | ||
| table.optionsStore.set(() => mergedOptions) | ||
| } else { | ||
| table.options = mergedOptions | ||
| } | ||
| table_syncExternalStateToBaseAtoms(table) | ||
| } |
Contributor
There was a problem hiding this comment.
Publish the options update and base-atom sync atomically.
In the optionsStore path, Line 69 can notify subscribers before Line 74 copies options.state into baseAtoms. That lets store-backed adapters briefly observe new options with stale table state.
Suggested fix
const newOptions = functionalUpdate(updater, table.options)
const mergedOptions = table_mergeOptions(table, newOptions)
- if (table.optionsStore) {
- table.optionsStore.set(() => mergedOptions)
- } else {
- table.options = mergedOptions
- }
- table_syncExternalStateToBaseAtoms(table)
+ table._reactivity.batch(() => {
+ if (table.optionsStore) {
+ table.optionsStore.set(() => mergedOptions)
+ } else {
+ table.options = mergedOptions
+ }
+ table_syncExternalStateToBaseAtoms(table)
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const newOptions = functionalUpdate(updater, table.options) | |
| const mergedOptions = table_mergeOptions(table, newOptions) | |
| table.optionsStore.set(() => mergedOptions) | |
| if (table.optionsStore) { | |
| table.optionsStore.set(() => mergedOptions) | |
| } else { | |
| table.options = mergedOptions | |
| } | |
| table_syncExternalStateToBaseAtoms(table) | |
| } | |
| const newOptions = functionalUpdate(updater, table.options) | |
| const mergedOptions = table_mergeOptions(table, newOptions) | |
| table._reactivity.batch(() => { | |
| if (table.optionsStore) { | |
| table.optionsStore.set(() => mergedOptions) | |
| } else { | |
| table.options = mergedOptions | |
| } | |
| table_syncExternalStateToBaseAtoms(table) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/table-core/src/core/table/coreTablesFeature.utils.ts` around lines
67 - 75, The current branch updates the external optionsStore which can notify
subscribers before base atoms are synchronized, causing adapters to see new
options with stale table state; to fix this, when table.optionsStore is present
compute mergedOptions via functionalUpdate and table_mergeOptions, then call
table_syncExternalStateToBaseAtoms(table) before publishing mergedOptions to
optionsStore (or perform the set with a callback that first syncs base atoms and
then returns mergedOptions), otherwise keep the existing fallback that assigns
table.options; reference optionsStore, functionalUpdate, table_mergeOptions,
table_syncExternalStateToBaseAtoms, and table.options to locate and adjust the
update order.
Comment on lines
25
to
33
| const tableState = tableInstance | ||
| ? useSelector(tableInstance.optionsStore, (state: unknown) => { | ||
| const { | ||
| state: _s, | ||
| data: _d, | ||
| _features: _f, | ||
| _rowModels: _r, | ||
| ...options | ||
| } = state as Record<string, unknown> | ||
| return options | ||
| }) | ||
| ? tableInstance.optionsStore | ||
| ? useSelector(tableInstance.optionsStore, (state: unknown) => | ||
| projectOptionsForTree(state), | ||
| ) | ||
| : useTableStore(tableInstance.store, () => | ||
| projectOptionsForTree(tableInstance.options as unknown), | ||
| ) | ||
| : undefined |
Contributor
There was a problem hiding this comment.
Options panel may display stale data for React/Preact tables when optionsStore is absent
The fallback branch (lines 30–32) subscribes to tableInstance.store (the state store) but reads tableInstance.options as a plain, non-reactive object. When a React/Preact table has createOptionsStore: false, table.options is updated via setOptions synchronously during render — but this does not notify table.store subscribers. As a result, option changes such as new data, columns, or meta references will not refresh the Options panel until the next table state change (e.g., a sort or selection), producing stale devtools output.
Consider capturing tableInstance.options at subscription time via a reactive accessor, or subscribing to a lightweight revision counter that setOptions increments, so the panel always reflects the latest options regardless of whether state changed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/table-devtools/src/components/OptionsPanel.tsx` around lines 25 -
33, The fallback branch currently subscribes to tableInstance.store but reads
the non-reactive tableInstance.options directly (in the useTableStore selector
passed to projectOptionsForTree), causing stale UI when createOptionsStore is
false; update the selector used in the useTableStore fallback to read a reactive
marker together with the options so the selector re-runs when options change —
e.g., read a lightweight revision or accessor exposed by the table (such as an
optionsRevision or getOptions accessor) inside the selector and then return
projectOptionsForTree(tableInstance.options) (or
projectOptionsForTree(tableInstance.getOptions())) so the OptionsPanel updates
whenever setOptions increments that revision or the accessor reflects new
options; ensure references to tableInstance, useTableStore,
projectOptionsForTree, tableInstance.options (or tableInstance.getOptions) and
the new revision/accessor are used in the selector.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…te subscriptions
🎯 Changes
✅ Checklist
pnpm test:pr.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores