Uh oh!
There was an error while loading. Please reload this page.
perf(table-core): pre-compute instance init functions - #6446
Merged
Conversation
When we construct a row, we iterate over the table's features and initialise any which have an `initRowInstanceData`. This is expensive since most don't have such a method. We actually know when we construct the table which features have this method. This changes the table construction to store the initialisers up front so when we create a row, we can just iterate the already known initialisers.
Contributor
📝 WalkthroughWalkthroughThe table core now precomputes feature instance-initialization callbacks for cells, columns, headers, header groups, and rows. Constructors invoke these callbacks, with tests covering lifecycle timing and caching. Framework guides and generated references document the expanded feature API. ChangesFeature instance initialization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/table-core/src/core/table/constructTable.ts`:
- Around line 71-79: Update the row-instance hook caching in the feature
iteration to preserve each feature as the callback receiver. Bind each
`initRowInstanceData` hook to its owning feature, or cache the feature/function
pair and invoke it with `feature` as `this` when populating
`table._rowInstanceInitFns`.
🪄 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: 35662c39-160b-4be9-bd3e-d6605d667753
📒 Files selected for processing (3)
packages/table-core/src/core/rows/constructRow.tspackages/table-core/src/core/table/constructTable.tspackages/table-core/src/core/table/coreTablesFeature.types.ts
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
KevinVandy
commented
Jul 21, 2026
Member
I like this change, and I will properly review this in a few days. |
View your CI Pipeline Execution ↗ for commit 7f6d788
☁️ Nx Cloud last updated this comment at |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/reference/static-functions/functions/selectRowsFn.md (1)
43-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPass the required table argument in the example.
The documented signature requires
selectRowsFn(rowModel, table), but the example callsselectRowsFn(rowModel). Update it to includetable.🤖 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/static-functions/functions/selectRowsFn.md` around lines 43 - 47, Update the selectRowsFn example to pass the required table argument, matching the documented selectRowsFn(rowModel, table) signature.
🧹 Nitpick comments (1)
packages/table-core/src/core/table/coreTablesFeature.types.ts (1)
164-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInstance-init-fn cache fields are always set but typed optional, forcing scattered
!assertions.
constructTableunconditionally assigns_cellInstanceInitFns/_columnInstanceInitFns/_headerGroupInstanceInitFns/_headerInstanceInitFns/_rowInstanceInitFnsfor every table (constructTable.ts lines 208-212), so they're always defined by the time cells/columns/headers/header-groups are constructed. Declaring them optional inTable_CorePropertiesforces every consumer to use a non-null assertion, which silently hides aTypeErrorrisk if this invariant is ever broken by a future code path.
packages/table-core/src/core/table/coreTablesFeature.types.ts#L164-L219: drop the?on all five_*InstanceInitFnsfields (make them required, defaulted to[]conceptually) so the type reflects the real invariant.packages/table-core/src/core/cells/constructCell.ts#L54-L59: drop the!ontable._cellInstanceInitFnsonce the field is non-optional.packages/table-core/src/core/columns/constructColumn.ts#L120-L123: drop the!ontable._columnInstanceInitFnsonce the field is non-optional.packages/table-core/src/core/headers/buildHeaderGroups.ts#L54-L55: drop the!ontable._headerGroupInstanceInitFnsonce the field is non-optional.packages/table-core/src/core/headers/constructHeader.ts#L66-L71: drop the!ontable._headerInstanceInitFnsonce the field is non-optional.♻️ Proposed type change
- _cellInstanceInitFns?: Array<+ _cellInstanceInitFns: Array< NonNullable<TableFeature['initCellInstanceData']> >🤖 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 164 - 219, Make the five instance-init-fn cache fields in packages/table-core/src/core/table/coreTablesFeature.types.ts#L164-219 required by removing their optional markers: _cellInstanceInitFns, _columnInstanceInitFns, _headerGroupInstanceInitFns, _headerInstanceInitFns, and _rowInstanceInitFns. Remove the corresponding non-null assertions in packages/table-core/src/core/cells/constructCell.ts#L54-59, packages/table-core/src/core/columns/constructColumn.ts#L120-123, packages/table-core/src/core/headers/buildHeaderGroups.ts#L54-55, and packages/table-core/src/core/headers/constructHeader.ts#L66-71; no direct change is required for _rowInstanceInitFns consumers at the cited sites.
🤖 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/index/interfaces/Table_Core.md`:
- Around line 29-61: Update the generated initializer-cache reference entries so
each Returns section declares void[] instead of void, matching the properties’
declared array return type. Apply this to the five listed ranges in
docs/reference/index/interfaces/Table_Core.md, the five listed ranges in
docs/reference/index/interfaces/Table_CoreProperties.md, and the five listed
ranges in docs/reference/index/interfaces/Table_Internal.md; no other
documentation changes are needed.
In `@docs/reference/index/interfaces/Table_Table.md`:
- Around line 30-66: Correct the five generated initializer-cache API entries,
including _cellInstanceInitFns and the entries at the referenced sections, to
document each property as an optional array of callback types matching the
upstream Array<NonNullable<TableFeature['init…']>> contract. Remove
the generated callable-property parameter and return sections, and preserve the
corresponding initializer callback names and feature types.
---
Outside diff comments:
In `@docs/reference/static-functions/functions/selectRowsFn.md`:
- Around line 43-47: Update the selectRowsFn example to pass the required table
argument, matching the documented selectRowsFn(rowModel, table) signature.
---
Nitpick comments:
In `@packages/table-core/src/core/table/coreTablesFeature.types.ts`:
- Around line 164-219: Make the five instance-init-fn cache fields in
packages/table-core/src/core/table/coreTablesFeature.types.ts#L164-219 required
by removing their optional markers: _cellInstanceInitFns,
_columnInstanceInitFns, _headerGroupInstanceInitFns, _headerInstanceInitFns, and
_rowInstanceInitFns. Remove the corresponding non-null assertions in
packages/table-core/src/core/cells/constructCell.ts#L54-59,
packages/table-core/src/core/columns/constructColumn.ts#L120-123,
packages/table-core/src/core/headers/buildHeaderGroups.ts#L54-55, and
packages/table-core/src/core/headers/constructHeader.ts#L66-71; no direct change
is required for _rowInstanceInitFns consumers at the cited sites.
🪄 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 Plus
Run ID: 0fb73ea2-bb9b-44db-82d9-7ea24ae00c74
📒 Files selected for processing (79)
docs/framework/alpine/guide/custom-features.mddocs/framework/angular/guide/custom-features.mddocs/framework/ember/guide/custom-features.mddocs/framework/lit/guide/custom-features.mddocs/framework/preact/guide/custom-features.mddocs/framework/react/guide/custom-features.mddocs/framework/solid/guide/custom-features.mddocs/framework/svelte/guide/custom-features.mddocs/framework/vue/guide/custom-features.mddocs/reference/index/functions/constructColumn.mddocs/reference/index/functions/makeStateUpdater.mddocs/reference/index/interfaces/ColumnDef_RowSorting.mddocs/reference/index/interfaces/FeatureSlotPrereqs.mddocs/reference/index/interfaces/Plugins.mddocs/reference/index/interfaces/Row_Aggregation.mddocs/reference/index/interfaces/TableFeature.mddocs/reference/index/interfaces/TableFeatures.mddocs/reference/index/interfaces/TableMeta.mddocs/reference/index/interfaces/TableOptions_Core.mddocs/reference/index/interfaces/TableOptions_Table.mddocs/reference/index/interfaces/Table_Core.mddocs/reference/index/interfaces/Table_CoreProperties.mddocs/reference/index/interfaces/Table_Internal.mddocs/reference/index/interfaces/Table_Table.mddocs/reference/index/type-aliases/Atoms.mddocs/reference/index/type-aliases/Atoms_All.mddocs/reference/index/type-aliases/BaseAtoms.mddocs/reference/index/type-aliases/BaseAtoms_All.mddocs/reference/index/type-aliases/BuiltInAggregationFn.mddocs/reference/index/type-aliases/ColumnDefResolved.mddocs/reference/index/type-aliases/DeepValue.mddocs/reference/index/type-aliases/ExternalAtoms.mddocs/reference/index/type-aliases/ExternalAtoms_All.mddocs/reference/index/type-aliases/ExtractFeatureMapTypes.mddocs/reference/index/type-aliases/ExtractTableMeta.mddocs/reference/index/type-aliases/IsAny.mddocs/reference/index/type-aliases/NonFeatureKeys.mddocs/reference/index/type-aliases/ValidateFeatureSlots.mddocs/reference/index/variables/aggregationFn_count.mddocs/reference/index/variables/aggregationFn_extent.mddocs/reference/index/variables/aggregationFn_first.mddocs/reference/index/variables/aggregationFn_last.mddocs/reference/index/variables/aggregationFn_max.mddocs/reference/index/variables/aggregationFn_mean.mddocs/reference/index/variables/aggregationFn_median.mddocs/reference/index/variables/aggregationFn_min.mddocs/reference/index/variables/aggregationFn_sum.mddocs/reference/index/variables/aggregationFn_unique.mddocs/reference/index/variables/aggregationFn_uniqueCount.mddocs/reference/index/variables/aggregationFns.mddocs/reference/static-functions/functions/aggregateColumnValue.mddocs/reference/static-functions/functions/cell_getIsAggregated.mddocs/reference/static-functions/functions/column_getAggregationFns.mddocs/reference/static-functions/functions/column_getAggregationValue.mddocs/reference/static-functions/functions/column_getAutoAggregationFn.mddocs/reference/static-functions/functions/formatAggregatedCellValue.mddocs/reference/static-functions/functions/isRowSelected.mddocs/reference/static-functions/functions/isSubRowSelected.mddocs/reference/static-functions/functions/normalizeUniqueAggregationRows.mddocs/reference/static-functions/functions/row_getCanMultiSelect.mddocs/reference/static-functions/functions/row_getCanSelect.mddocs/reference/static-functions/functions/row_getCanSelectSubRows.mddocs/reference/static-functions/functions/row_getIsAllSubRowsSelected.mddocs/reference/static-functions/functions/row_getIsSelected.mddocs/reference/static-functions/functions/row_getIsSomeSelected.mddocs/reference/static-functions/functions/row_getToggleSelectedHandler.mddocs/reference/static-functions/functions/row_toggleSelected.mddocs/reference/static-functions/functions/selectRowsFn.mddocs/reference/static-functions/index.mdpackages/table-core/src/core/cells/constructCell.tspackages/table-core/src/core/columns/constructColumn.tspackages/table-core/src/core/headers/buildHeaderGroups.tspackages/table-core/src/core/headers/constructHeader.tspackages/table-core/src/core/table/constructTable.tspackages/table-core/src/core/table/coreTablesFeature.types.tspackages/table-core/src/types/TableFeatures.tspackages/table-core/tests/unit/core/cells/constructCell.test.tspackages/table-core/tests/unit/core/headers/constructHeader.test.tspackages/table-core/tests/unit/core/table/constructTable.test.ts
Comment on lines
+29
to
+61
| ### \_cellInstanceInitFns? | ||
| ```ts | ||
| optional _cellInstanceInitFns: <TFeatures, TData, TValue>(cell) => void[]; | ||
| ``` | ||
| Defined in: [core/table/coreTablesFeature.types.ts:167](https://github.com/TanStack/table/blob/main/packages/table-core/src/core/table/coreTablesFeature.types.ts#L167) | ||
| Cache of the `initCellInstanceData` functions for features that define one. | ||
| #### Type Parameters | ||
| ##### TFeatures | ||
| `TFeatures` *extends* [`TableFeatures`](TableFeatures.md) | ||
| ##### TData | ||
| `TData` *extends* [`RowData`](../type-aliases/RowData.md) | ||
| ##### TValue | ||
| `TValue` *extends* `unknown` = `unknown` | ||
| #### Parameters | ||
| ##### cell | ||
| [`Cell`](../type-aliases/Cell.md)\<`TFeatures`, `TData`, `TValue`\> | ||
| #### Returns | ||
| `void` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"Locate referenced source and docs files"forfin \
docs/reference/index/interfaces/Table_Core.md \
docs/reference/index/interfaces/Table_CoreProperties.md \
docs/reference/index/interfaces/Table_Internal.md \
packages/table-core/src/core/table/coreTablesFeature.types.ts
doif [ -f"$f" ];thenecho"FOUND $f"elseecho"MISSING $f"fidoneechoecho"Relevant source declarations"if [ -f packages/table-core/src/core/table/coreTablesFeature.types.ts ];then
sed -n '140,230p' packages/table-core/src/core/table/coreTablesFeature.types.ts | nl -ba -v140
fiechoecho"Docs snippets around Returns for one cache per file (using _cellInstanceInitFns as representative)"forfin \
docs/reference/index/interfaces/Table_Core.md \
docs/reference/index/interfaces/Table_CoreProperties.md \
docs/reference/index/interfaces/Table_Internal.md
doecho"--- $f"
sed -n '24,65p'"$f"doneechoecho"All initializer-cache return declarations and docs returns"
python3 - <<'PY'from pathlib import Pathimport refiles = [ Path('docs/reference/index/interfaces/Table_Core.md'), Path('docs/reference/index/interfaces/Table_CoreProperties.md'), Path('docs/reference/index/interfaces/Table_Internal.md'),]source = Path('packages/table-core/src/core/table/coreTablesFeature.types.ts')patterns = [ re.compile(r'optional (_cellInstanceInitFns|_columnInstanceInitFns|_headerGroupInstanceInitFns|_headerInstanceInitFns|_rowInstanceInitFns)\?:\s*<([^>]+(?:>[^>]+)*)>\([^)]*\)\s*=>\s*([^\n;]+);?'), re.compile(r'###\s+\\_?(\w+InstanceInitFns)\?[\s\S]*?Returns?\s*`([^\n`]*)`', re.I),]print("Source declarations")if source.exists(): text = source.read_text() for m in patterns[0].finditer(text): name, typepars, ret = m.groups() ret = ret.strip() # collapse spaces, remove semicolon ret = re.sub(r'\s+', ' ', ret).strip().rstrip(';') print(f"{name}: returns {ret}")print("\nDocs returns")for f in files: if not f.exists(): print(f"MISSING {f}") continue text = f.read_text() for m in patterns[1].finditer(text): name, ret = m.groups() print(f"{f}:{text[:m.start()].count(chr(10))+1}-{text[:m.end()].count(chr(10))}: {name} returns {ret}")print("\nDeterministic signature/returns text check")for f in files: text = f.read_text().replace(' ', '').replace('\n', '') cache_names = ['CellInstanceInitFns','ColumnInstanceInitFns','HeaderGroupInstanceInitFns','HeaderInstanceInitFns','RowInstanceInitFns'] for name in cache_names: # simple tuple: signature appears at least once, returns appears at least once, docs return exact void sig = f'{name}?`<TFeatures,TData' in text has_returns = f'Returns`{name}?' in text or 'Returns`\n\n###' not in text returns_void = f'Returns`void' in text returns_voidarray = f'Returns`void[]' in text signature_void = f'{name}?:<[{name[0].lower()}{name[1:-len("Fns"):-1]}' in text and '=>void[];' in text or False print(f"{f}: {name} signature_voidarray={signature_void} docs_returns_void={returns_void} docs_returns_voidarray={returns_voidarray}")PYRepository: TanStack/table
Length of output: 501
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"Relevant source declarations"if [ -f packages/table-core/src/core/table/coreTablesFeature.types.ts ];then
sed -n '140,230p' packages/table-core/src/core/table/coreTablesFeature.types.ts
fiechoecho"Docs snippets around returns for _cellInstanceInitFns in each affected interface"forfin \
docs/reference/index/interfaces/Table_Core.md \
docs/reference/index/interfaces/Table_CoreProperties.md \
docs/reference/index/interfaces/Table_Internal.md
doecho"=== $f ==="
sed -n '24,65p'"$f"doneechoecho"All initializer-cache return declarations and docs Returns values"
python3 - <<'PY'from pathlib import Pathimport refiles = [ Path('docs/reference/index/interfaces/Table_Core.md'), Path('docs/reference/index/interfaces/Table_CoreProperties.md'), Path('docs/reference/index/interfaces/Table_Internal.md'),]source = Path('packages/table-core/src/core/table/coreTablesFeature.types.ts')patterns = [ re.compile(r'optional (_cellInstanceInitFns|_columnInstanceInitFns|_headerGroupInstanceInitFns|_headerInstanceInitFns|_rowInstanceInitFns)\?:\s*<([^>]+(?:>[^>]+)*)>\([^)]*\)\s*=>\s*([^\n;]+);?'), re.compile(r'(###\s+\\?\w+InstanceInitFns\?)([\s\S]*?)####\s+Returns\n\n```ts\noptional', re.I),]print("Source declarations")if source.exists(): text = source.read_text() for m in patterns[0].finditer(text): name, _, ret = m.groups() print(f"{name}: {ret.strip()}")print("\nDocs returns")for f in files: if not f.exists(): print(f"MISSING {f}") continue text = f.read_text() for block in patterns[1].finditer(text): head = block.group(1).replace('\\', '').strip() body = block.group(2) ret_match = re.search(r'Returns\n\n```ts\noptional\s+\w+InstanceInitFns\?\?\??:\s*<([^>]+(?:>[^>]+)*)>\([^)]*\)\s*=>\s*([^\n;]+);', body) if ret_match: _, ret = ret_match.groups() print(f"{f}:{line_number(text, head)}-{last_line_number(block[0], text)}: {head} signature returns {ret.strip()}")def line_number(haystack, needle): idx = haystack.find(needle) if idx == -1: return 0 return haystack[:idx].count('\n') + 1def last_line_number(re_match, text): return text[:re_match.end()].count('\n') + 1print("\nDetailed doc return lines")for f in files: lines = f.read_text().splitlines() for needle in ['Returns', 'Returns:']: # find lines close enough where return type appears on next code line for i,l in enumerate(lines): if l.strip().lower() == (needle.lower() if needle == 'Returns:' else l.strip().lower()): print(f"{f}:{i+1}: {l}") for j in range(i, min(i+12, len(lines))): print(f"{f}:{j+1}: {lines[j]}") if '```' in lines[j] and j > i: breakPYRepository: tm
Length of output: 1991
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"Relevant source declarations"if [ -f packages/table-core/src/core/table/coreTablesFeature.types.ts ];then
sed -n '140,230p' packages/table-core/src/core/table/coreTablesFeature.types.ts
fiechoecho"Doc content for _cellInstanceInitFns in affected interfaces"forfin \
docs/reference/index/interfaces/Table_Core.md \
docs/reference/index/interfaces/Table_CoreProperties.md \
docs/reference/index/interfaces/Table_Internal.md
doecho"=== $f ==="
sed -n '1,70p'"$f"doneechoecho"Search initializer-cache declarations and generated return blocks"
rg -n "InstanceInitFns|Returns" docs/reference/index/interfaces/Table_Core.md docs/reference/index/interfaces/Table_CoreProperties.md docs/reference/index/interfaces/Table_Internal.md packages/table-core/src/core/table/coreTablesFeature.types.ts ||trueRepository: TanStack/table
Length of output: 20808
Fix the initializer-cache return values in generated references.
The generated docs show these properties returning void[], while the #### Returns sections say void. Align the return docs with the declared array contract in the affected generated files.
🧰 Tools
🪛 LanguageTool
[grammar] ~41-~41: Ensure spelling is correct
Context: ...efine one. #### Type Parameters ##### TFeatures TFeaturesextends [TableFeatures](TableFeatur...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~45-~45: Ensure spelling is correct
Context: ...ableFeatures](TableFeatures.md) ##### TData TData *extends* [RowData`](../type-aliases/R...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~49-~49: Ensure spelling is correct
Context: ...ta](../type-aliases/RowData.md) ##### TValue TValue*extends*unknown=unknown` #### P...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
📍 Affects 3 files
docs/reference/index/interfaces/Table_Core.md#L29-L61(this comment)docs/reference/index/interfaces/Table_Core.md#L85-L117docs/reference/index/interfaces/Table_Core.md#L157-L185docs/reference/index/interfaces/Table_Core.md#L193-L225docs/reference/index/interfaces/Table_Core.md#L265-L293docs/reference/index/interfaces/Table_CoreProperties.md#L26-L58docs/reference/index/interfaces/Table_CoreProperties.md#L74-L106docs/reference/index/interfaces/Table_CoreProperties.md#L134-L162docs/reference/index/interfaces/Table_CoreProperties.md#L166-L198docs/reference/index/interfaces/Table_CoreProperties.md#L226-L254docs/reference/index/interfaces/Table_Internal.md#L28-L60docs/reference/index/interfaces/Table_Internal.md#L86-L118docs/reference/index/interfaces/Table_Internal.md#L162-L190docs/reference/index/interfaces/Table_Internal.md#L200-L232docs/reference/index/interfaces/Table_Internal.md#L274-L302
🤖 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/index/interfaces/Table_Core.md` around lines 29 - 61, Update
the generated initializer-cache reference entries so each Returns section
declares void[] instead of void, matching the properties’ declared array return
type. Apply this to the five listed ranges in
docs/reference/index/interfaces/Table_Core.md, the five listed ranges in
docs/reference/index/interfaces/Table_CoreProperties.md, and the five listed
ranges in docs/reference/index/interfaces/Table_Internal.md; no other
documentation changes are needed.
Comment on lines
+30
to
+66
| ### \_cellInstanceInitFns? | ||
| ```ts | ||
| optional _cellInstanceInitFns: <TFeatures, TData, TValue>(cell) => void[]; | ||
| ``` | ||
| Defined in: [core/table/coreTablesFeature.types.ts:167](https://github.com/TanStack/table/blob/main/packages/table-core/src/core/table/coreTablesFeature.types.ts#L167) | ||
| Cache of the `initCellInstanceData` functions for features that define one. | ||
| #### Type Parameters | ||
| ##### TFeatures | ||
| `TFeatures` *extends* [`TableFeatures`](TableFeatures.md) | ||
| ##### TData | ||
| `TData` *extends* [`RowData`](../type-aliases/RowData.md) | ||
| ##### TValue | ||
| `TValue` *extends* `unknown` = `unknown` | ||
| #### Parameters | ||
| ##### cell | ||
| [`Cell`](../type-aliases/Cell.md)\<`TFeatures`, `TData`, `TValue`\> | ||
| #### Returns | ||
| `void` | ||
| #### Inherited from | ||
| [`Table_CoreProperties`](Table_CoreProperties.md).[`_cellInstanceInitFns`](Table_CoreProperties.md#_cellinstanceinitfns) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Document initializer caches as arrays, not callable properties.
The upstream contract defines these properties as optional arrays of callbacks, e.g. Array<NonNullable<TableFeature['initCellInstanceData']>>. The generated entries currently render each cache as a generic function returning void[], including incorrect parameter/return sections. Regenerate or correct these five API entries.
Also applies to: 86-122, 158-190, 194-230, 266-298
🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: Ensure spelling is correct
Context: ...efine one. #### Type Parameters ##### TFeatures TFeaturesextends [TableFeatures](TableFeatur...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~46-~46: Ensure spelling is correct
Context: ...ableFeatures](TableFeatures.md) ##### TData TData *extends* [RowData`](../type-aliases/R...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~50-~50: Ensure spelling is correct
Context: ...ta](../type-aliases/RowData.md) ##### TValue TValue*extends*unknown=unknown` #### P...
(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/index/interfaces/Table_Table.md` around lines 30 - 66, Correct
the five generated initializer-cache API entries, including _cellInstanceInitFns
and the entries at the referenced sections, to document each property as an
optional array of callback types matching the upstream
Array<NonNullable<TableFeature['init…']>> contract. Remove the
generated callable-property parameter and return sections, and preserve the
corresponding initializer callback names and feature types.
Uh oh!
There was an error while loading. Please reload this page.
KevinVandy added a commit
that referenced
this pull request
Jul 28, 2026
…s 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 <noreply@anthropic.com>
KevinVandy added a commit
that referenced
this pull request
Jul 28, 2026
…gs (#6459) * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * fix: adopt render-phase reactivity in the lit adapter, un-pin subscribe islands from stale renders 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * ci: apply automated fixes * consolidate constructTable features loop again * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
KevinVandy added a commit
that referenced
this pull request
Jul 29, 2026
* fix: react adapter sync state * Some changes * refactor: move render-phase sync strategy into core reactivity bindings (#6459) * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * fix: adopt render-phase reactivity in the lit adapter, un-pin subscribe islands from stale renders 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * ci: apply automated fixes * consolidate constructTable features loop again * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * clean up * fix some issues exposed by MRT examples * add more react smoke tests --------- Co-authored-by: Kevin Van Cott <kevinvandy656@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This was referenced Jul 30, 2026
Merged
Merged
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When we construct a row, we iterate over the table's features and initialise any which have an
initRowInstanceData.This is expensive since most don't have such a method.
We actually know when we construct the table which features have this method. This changes the table construction to store the initialisers up front so when we create a row, we can just iterate the already known initialisers.
✅ Checklist
pnpm test:pr.Summary by CodeRabbit