TanStack / table

🤖 Headless UI for building powerful tables & datagrids for TS/JS - React-Table, Vue-Table, Solid-Table, Svelte-Table
https://tanstack.com/table
MIT License
24.49k stars 3.03k forks source link

cell.getIsAggregated should not check for zero length subrows #5605

Open tmax22 opened 2 weeks ago

tmax22 commented 2 weeks ago

TanStack Table version

table-core v8.17.3

Framework/Library version

react

Describe the bug and the steps to reproduce it

current implementation for cell.getIsAggregated is

https://github.com/TanStack/table/blob/22e1ac4e8536fb959093574b0e2d42d3faa6f992/packages/table-core/src/features/ColumnGrouping.ts#L396

i would argue that row.subRows?.length is not supposed to be there.

look:

image

as we can see, aggregated cells exists even if no subrows exists for this group. this happens on custom getGroupedRowModel that i implement that makes sure that certain grouping values always exists even if there is no current rows for them(full explanation).

this is not a bug, but rather would say unexpected behavior for the method cell.getIsAggregated. i would argue that at least the next major release should change this behavior.

in my case, checking for row.depth === 0 instead is enough:

const customIsAggregated = !cell.getIsGrouped() && !cell.getIsPlaceholder() && row.depth === 0;
  let renderedCellValue =
    customIsAggregated && columnDef.AggregatedCell
...

image

Your Minimal, Reproducible Example - (Sandbox Highly Recommended)

.

Screenshots or Videos (Optional)

No response

Do you intend to try to help solve this bug with your own PR?

Yes, I think I know how to fix it and will discuss it in the comments of this issue

Terms & Code of Conduct

KevinVandy commented 5 days ago

so like a cell.getIsAggregationCell and similar APIs?

tmax22 commented 5 days ago

If you are suggesting adding new api cell.getIsAggregationCell, so yeah, possibly. But when when you would want to use cell.getIsAggregated?