Skip to content

fix: type aggregatedCell against bound cellComponents in createAppColumnHelper - #6589

Open
cpruijsen wants to merge 1 commit into
TanStack:mainfrom
cpruijsen:fix/issue-6583
Open

fix: type aggregatedCell against bound cellComponents in createAppColumnHelper#6589
cpruijsen wants to merge 1 commit into
TanStack:mainfrom
cpruijsen:fix/issue-6583

Conversation

@cpruijsen

@cpruijsen cpruijsen commented Sep 11, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

Changes

aggregatedCell is now omitted and redeclared against AppCellContext, matching cell, on AppColumnDefBase, AppDisplayColumnDef, and AppGroupColumnDef. The same copy of those types exists in every adapter with a component registry (React, Preact, Solid, Svelte, Vue, Lit, Angular, Octane), so all of them are updated. Alpine and Ember do not bind cell components on this type and are unchanged. Type-only; no runtime change.

createAppColumnHelper already typed cell, header, and footer against the registered cellComponents / headerComponents. aggregatedCell (from rowAggregationFeature) uses the same cell context at runtime: AppCell assigns those components onto the cell instance, and FlexRender calls aggregatedCell with cell.getContext(). Its column-def type stayed on the plain Cell. Bound names such as cell.NameCell type-checked in cell and failed in aggregatedCell.

Fixes #6583

The omit-and-redeclare is unconditional, matching cell / header / footer. Closed #6584 was React-only; the other option is to show aggregatedCell only when rowAggregationFeature is in TFeatures (core feature-gates the key; this type does not). aggregatedCell is the only other CellContext renderer. Can drop the extra adapters or gate the key on the feature.

Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested code changes locally with pnpm test and pnpm test:e2e, or these tests do not apply to this pull request.
  • I fully understand the code in this pull request, including any code generated with AI assistance.

Package test:types was run for the eight affected adapters; @tanstack/react-table test:lib (34 tests) and test:eslint also passed. End-to-end tests do not apply.

Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • New Features

    • Added support for defining custom aggregated-cell templates in application column definitions across Angular, Lit, Octane, Preact, React, Solid, Svelte, and Vue integrations.
    • Aggregated-cell templates now receive the same registered cell components and context available to standard cell templates.

  • Bug Fixes

    • Corrected aggregated-cell typing to match bound cell-component behavior.

@cpruijsen
fix: type aggregatedCell against bound cellComponents in createAppCol…
d91d72e 
…umnHelper

AppColumnDefBase already omitted and retyped cell/header/footer so registered
cellComponents appear on the render context. aggregatedCell uses the same cell
context at runtime but was left on the core type, so bound components type-checked
inside cell and failed inside aggregatedCell.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 53a134da-6591-49d4-98ac-afefc8c9ffc9

📥 Commits

Reviewing files that changed from the base of the PR and between ce123bc and d91d72e.

📒 Files selected for processing (10)
  • .changeset/bound-aggregated-cell-components.md
  • packages/angular-table/src/helpers/createTableHook.ts
  • packages/lit-table/src/createTableHook.ts
  • packages/octane-table/src/types.ts
  • packages/preact-table/src/createTableHook.tsx
  • packages/react-table/src/createTableHook.tsx
  • packages/react-table/tests/createAppColumnHelper.test-d.tsx
  • packages/solid-table/src/createTableHook.tsx
  • packages/svelte-table/src/createTableHook.svelte.ts
  • packages/vue-table/src/createTableHook.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

This change updates app column definition types across eight table packages. aggregatedCell now uses the same bound cellComponents context as cell. A React type-level test covers accessor, display, and group helpers. Patch releases are declared.

Changes

Aggregated cell component typing

Layer / File(s) Summary
Column definition type updates
packages/angular-table/..., packages/lit-table/..., packages/octane-table/..., packages/preact-table/..., packages/react-table/..., packages/solid-table/..., packages/svelte-table/..., packages/vue-table/...		 AppColumnDefBase, AppDisplayColumnDef, and AppGroupColumnDef omit and re-declare aggregatedCell with enhanced AppCellContext types. Documentation comments include the aggregated-cell context.
Type-level validation and release metadata
packages/react-table/tests/createAppColumnHelper.test-d.tsx, .changeset/bound-aggregated-cell-components.md		 Type-level tests cover bound and unregistered components across column helpers. The changeset declares patch releases for eight packages.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: kevinvandy

Merge Risk: ⚪ Minimal · up to d91d7

The type updates consistently bind registered cell components for aggregated cells without changing runtime behavior. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 9 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: typing aggregatedCell against bound cellComponents in createAppColumnHelper.
Description check ✅ Passed The description explains the change, affected adapters, motivation, implementation choice, testing, checklist status, and changeset release impact. The headings differ slightly from the template, but …
Linked Issues check ✅ Passed The PR satisfies #6583 and #6584. It omits and redeclares aggregatedCell with AppCellContext in AppColumnDefBase, AppDisplayColumnDef, and AppGroupColumnDef for Angular, Lit, Octane, Preact,…
Out of Scope Changes check ✅ Passed The changes are limited to the requested aggregatedCell type enhancements, related documentation comments, type-level coverage, and a changeset for the affected packages. These changes directly supp…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 9 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

aggregatedCell not typed against bound cellComponents in createAppColumnHelper

1 participant

@cpruijsen