Grouping by not using subRows

rasgo-cc commented 4 weeks ago

TanStack Table version

8.17.3

Framework/Library version

React v18.2.0

Describe the bug and the steps to reproduce it

Not sure if a bug of if it's intended behavior but the grouping feature doesn't seem to look at values in subRows. Please check the codesandbox below. I'd expect to have "John (4)", and to actually have all rows and subRows grouped by firstName but only the top-level rows are grouped by firstName.

If this is intended behavior, a consequence is that firstName in the subRows is empty and misleading (e.g. "John Hey" shows up under "James" but only "Hey" is visible). Still, in case this is still intended behavior, how could this be avoided so the table groups by a column in all rows and subRows?

Your Minimal, Reproducible Example - (Sandbox Highly Recommended)

On the sandbox below, click on the "First Name" group icon first, and then on the button "Toggle expand all" button below the table. This way we ensure the table is grouped by firstName and all rows are expanded.

https://codesandbox.io/p/sandbox/sharp-meninsky-766zsz?file=%2Fsrc%2FApp.tsx%3A62%2C26

Screenshots or Videos (Optional)

No response

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

Maybe, I'll investigate and start debugging

Terms & Code of Conduct

[X] I agree to follow this project's Code of Conduct
[X] I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.

rasgo-cc commented 4 weeks ago

In case you confirm this to be an issue/not intended behavior, it seems the culprit is here: https://github.com/TanStack/table/blob/main/packages/table-core/src/utils/getGroupedRowModel.ts#L168

The groupBy function only handles top-level rows. I was able to go around this and implement desired behavior (i.e. values from all rows and subRows get grouped by) by making it recursive like this:

function groupBy<TData extends RowData>(
  rows: Row<TData>[],
  columnId: string,
  opts: Partial<{
    groupMap: Map<any, Row<TData>[]>;
  }> = {}
) {
  const groupMap = opts.groupMap ?? new Map<any, Row<TData>[]>();

  return rows.reduce((map, row) => {
    const resKey = `${row.getGroupingValue(columnId)}`;
    const previous = map.get(resKey);
    if (!previous) {
      map.set(resKey, [row]);
    } else {
      previous.push(row);
    }
    if (row.subRows) map = groupBy(row.subRows, columnId, { groupMap: map });
    return map;
  }, groupMap);
}

Looking forward to know what you think of this. This would probably be a breaking change for some? Since we can pass a getGroupedRowModel function to useReactTable, that's probably what I'll do for the time being. However, maybe a table option could be added to allow this behavior?

dima5513 commented 2 weeks ago

I need to display tabular data by groups so that above each group there is a row that is the group header and I would also like to add a button to hide the group in it, but the virtualizer obliges me not to group the data and go through rows and not leaf or flat rows. Otherwise, it doesn't work properly. Are there any examples that I could look at? The example in the screenshot was taken from examples -> subComponent, but I can't reproduce this using c virtualizer

TanStack / table