fix(Table): keep group headers out of the caller's cell renderers

.changeset/grouped-rows-header-cell-guard.md

@@ -0,0 +1,9 @@
+---
+'@astryxdesign/core': patch
+---
+
+[fix] useTableGroupedRows: stop running column `renderCell` functions against synthetic group-header rows, which was crashing any table whose renderer keys a lookup off a field. The header Proxy resolving unknown fields to `''` only ever rescued renderers that _print_ a field — `STATUS_META[item.status].dot` throws on `''` exactly as it would on `undefined`, and BaseTable evaluates every column's `renderCell` on every row before `transformBodyRow` can discard a header's cells. The plugin now skips those calls outright; the cells were being thrown away moments later regardless.
+
+Also returns `isGroupHeader` from the hook, so consumers can guard their own row-level plugins and handlers (click-to-open-detail, row links, per-row menus) without string-matching the `__group_` key prefix.
+
+@ernesttien

packages/core/src/Table/plugins/groupedRows/useTableGroupedRows.test.tsx

@@ -175,6 +175,130 @@ describe('useTableGroupedRows', () => {
     ).not.toBeInTheDocument();
   });
 
+  it('does not run a column renderCell against a group header row', () => {
+    // The realistic shape: a renderer that keys a lookup off a field rather
+    // than printing it. The header Proxy answers `''`, which is not a member
+    // of the map, so reaching into the result throws unless the header is
+    // skipped outright.
+    const TEAM_META: Record<string, {dot: string}> = {
+      Core: {dot: 'green'},
+      Infra: {dot: 'blue'},
+    };
+    const seen: Person[] = [];
+    const lookupColumns: TableColumn<Person>[] = [
+      {
+        key: 'team',
+        header: 'Team',
+        renderCell: item => {
+          seen.push(item);
+          return <span>{TEAM_META[item.team].dot}</span>;
+        },
+      },
+    ];
+
+    function LookupHarness() {
+      const [collapsed, setCollapsed] = useState<Set<string>>(new Set());
+      const grouped = useTableGroupedRows<Person>({
+        data: people,
+        groupBy: p => p.team,
+        collapsedGroups: collapsed,
+        onToggleGroup: k => setCollapsed(new Set([k])),
+        getRowKey: p => p.id,
+      });
+      return (
+        <Table
+          data={grouped.data}
+          columns={lookupColumns}
+          idKey={grouped.idKey}
+          plugins={{grouped: grouped.plugin}}
+        />
+      );
+    }
+
+    expect(() => render(<LookupHarness />)).not.toThrow();
+    // Only the three real rows reached the renderer.
+    expect(seen).toHaveLength(3);
+    expect(seen.map(p => p.name)).toEqual(['Alice', 'Bob', 'Carol']);
+    // Group headers still render their own label.
+    expect(screen.getByText('Core')).toBeInTheDocument();
+    expect(screen.getByText('Infra')).toBeInTheDocument();
+  });
+
+  it('returns the same wrapped column objects across calls', () => {
+    // BaseTable compares the resolved column array element by element to
+    // decide whether every row needs re-rendering, so the guard must not
+    // allocate a fresh column on each render.
+    const stableColumns: TableColumn<Person>[] = [
+      {key: 'name', header: 'Name', renderCell: item => <>{item.name}</>},
+      {key: 'team', header: 'Team'},
+    ];
+    let transform:
+      ((c: TableColumn<Person>[]) => TableColumn<Person>[]) | null = null;
+    function StabilityHarness() {
+      const [collapsed, setCollapsed] = useState<Set<string>>(new Set());
+      const grouped = useTableGroupedRows<Person>({
+        data: people,
+        groupBy: p => p.team,
+        collapsedGroups: collapsed,
+        onToggleGroup: k => setCollapsed(new Set([k])),
+        getRowKey: p => p.id,
+      });
+      transform = grouped.plugin.transformColumns ?? null;
+      return (
+        <Table
+          data={grouped.data}
+          columns={stableColumns}
+          idKey={grouped.idKey}
+          plugins={{grouped: grouped.plugin}}
+        />
+      );
+    }
+    render(<StabilityHarness />);
+    expect(transform).not.toBeNull();
+    const first = transform!(stableColumns);
+    const second = transform!(stableColumns);
+    expect(first[0]).toBe(second[0]);
+    // A column with no renderCell needs no wrapper at all.
+    expect(first[1]).toBe(stableColumns[1]);
+  });
+
+  it('exposes isGroupHeader to distinguish synthetic rows from real ones', () => {
+    const calls: {key: string; header: boolean}[] = [];
+    function PredicateHarness() {
+      const [collapsed, setCollapsed] = useState<Set<string>>(new Set());
+      const grouped = useTableGroupedRows<Person>({
+        data: people,
+        groupBy: p => p.team,
+        collapsedGroups: collapsed,
+        onToggleGroup: k => setCollapsed(new Set([k])),
+        getRowKey: p => p.id,
+      });
+      calls.length = 0;
+      for (const row of grouped.data) {
+        calls.push({
+          key: grouped.idKey(row),
+          header: grouped.isGroupHeader(row),
+        });
+      }
+      return (
+        <Table
+          data={grouped.data}
+          columns={columns}
+          idKey={grouped.idKey}
+          plugins={{grouped: grouped.plugin}}
+        />
+      );
+    }
+    render(<PredicateHarness />);
+    expect(calls).toEqual([
+      {key: '__group_Core', header: true},
+      {key: 'a', header: false},
+      {key: 'b', header: false},
+      {key: '__group_Infra', header: true},
+      {key: 'c', header: false},
+    ]);
+  });
+
   it('keeps a group collapsed across a data change (state keyed by group)', () => {
     function ChangingHarness() {
       const [collapsed, setCollapsed] = useState<Set<string>>(

packages/core/src/Table/plugins/groupedRows/useTableGroupedRows.tsx

@@ -20,7 +20,7 @@ import {
   fontWeightVars,
 } from '../../../theme/tokens.stylex';
 import {Icon} from '../../../Icon';
-import type {TablePlugin} from '../../types';
+import type {TableColumn, TablePlugin} from '../../types';
 import {useTranslator} from '../../../i18n';
 
 // A synthetic group-header row injected into the flattened data. Real rows
@@ -41,8 +41,10 @@ function isGroupHeader(item: unknown): item is GroupHeader {
   );
 }
 
-// Proxy handler: any field access beyond the marker fields resolves to `''`
-// so user cell renderers (`item.name.toUpperCase()`) never throw on a header.
+// Proxy handler: any field access beyond the marker fields resolves to `''`,
+// which keeps the default cell renderer (`String(item[key])`) harmless on a
+// header row. It is only a backstop — `transformColumns` below stops user
+// `renderCell` functions from seeing a header row at all.
 const HEADER_PROXY_HANDLER: ProxyHandler<Record<string | symbol, unknown>> = {
   // eslint-disable-next-line @typescript-eslint/promise-function-async -- Proxy get trap, not a promise-returning fn
   get(t: Record<string | symbol, unknown>, prop: string | symbol): unknown {
@@ -54,12 +56,15 @@ const HEADER_PROXY_HANDLER: ProxyHandler<Record<string | symbol, unknown>> = {
 };
 
 /**
- * Build a synthetic header row wrapped in a Proxy so arbitrary field access
- * from user cell renderers (e.g. `item.name.toUpperCase()`) resolves to `''`
- * instead of throwing — BaseTable evaluates `col.renderCell(item)` on every
- * row (including synthetic headers) before `transformBodyRow` can replace the
- * row's cells. `transformBodyRow` then discards those cells and renders a
- * single full-width header cell.
+ * Build a synthetic header row wrapped in a Proxy so the default cell renderer
+ * (`String(item[key])`) reads `''` on a header instead of `undefined`.
+ *
+ * The Proxy alone is not enough to protect user code: it rescues a renderer
+ * that *prints* a field, but not the far more common one that *keys off* it
+ * (`STATUS_META[item.status].dot` throws on `''` just as it would on
+ * `undefined`). `transformColumns` therefore skips `renderCell` entirely for
+ * header rows, and `transformBodyRow` replaces the row's cells with a single
+ * full-width header cell.
  */
 function makeHeader<T extends Record<string, unknown>>(
   groupKey: string,
@@ -73,6 +78,37 @@ function makeHeader<T extends Record<string, unknown>>(
   return new Proxy(target, HEADER_PROXY_HANDLER) as unknown as T;
 }
 
+// Wrapping a column allocates a new object, and BaseTable decides whether
+// every row must re-render by comparing the resolved column array element by
+// element. Cache the wrapper against its source column so a stable `columns`
+// prop keeps yielding the very same objects and that check keeps passing.
+const headerSafeColumns = new WeakMap<object, unknown>();
+
+/**
+ * A column whose `renderCell` skips synthetic group-header rows. Columns
+ * without one are returned untouched: the default renderer reads `item[key]`,
+ * which the header Proxy already answers with `''`.
+ */
+function headerSafeColumn<T extends Record<string, unknown>>(
+  column: TableColumn<T>,
+): TableColumn<T> {
+  const {renderCell} = column;
+  if (!renderCell) {
+    return column;
+  }
+  const cached = headerSafeColumns.get(column);
+  if (cached) {
+    return cached as TableColumn<T>;
+  }
+  const wrapped: TableColumn<T> = {
+    ...column,
+    renderCell: (item: T): ReactNode =>
+      isGroupHeader(item) ? null : renderCell(item),
+  };
+  headerSafeColumns.set(column, wrapped);
+  return wrapped;
+}
+
 /** Configuration for {@link useTableGroupedRows}. */
 export interface UseTableGroupedRowsConfig<T extends Record<string, unknown>> {
   /** The flat data to group. */
@@ -109,6 +145,19 @@ export interface UseTableGroupedRowsResult<T extends Record<string, unknown>> {
    * `<Table idKey={grouped.idKey} />`.
    */
   idKey: (item: T) => string;
+  /**
+   * True when the row is a synthetic group header rather than one of your own
+   * rows. Row-level plugins and handlers see both, so guard anything that
+   * assumes a real row — click-to-open-detail, row links, per-row menus:
+   *
+   * ```
+   * transformBodyRow(props, item) {
+   *   if (grouped.isGroupHeader(item)) return props;
+   *   return {...props, htmlProps: {...props.htmlProps, onClick: () => open(item)}};
+   * }
+   * ```
+   */
+  isGroupHeader: (item: T) => boolean;
 }
 
 const styles = stylex.create({
@@ -295,6 +344,14 @@ export function useTableGroupedRows<T extends Record<string, unknown>>(
 
   const plugin = useMemo(
     (): TablePlugin<T> => ({
+      // BaseTable evaluates every column's `renderCell` against every row
+      // before `transformBodyRow` gets to discard a header's cells. A header
+      // is not one of the caller's rows, so running their renderer against it
+      // can only fail — the cells produced here are thrown away moments later
+      // regardless. Skip the call and hand back the empty cell directly.
+      transformColumns(columns) {
+        return columns.map(headerSafeColumn);
+      },
       // Replace a header row's pre-rendered cells with one full-width cell.
       transformBodyRow(props, item) {
         if (!isGroupHeader(item)) {
@@ -371,5 +428,5 @@ export function useTableGroupedRows<T extends Record<string, unknown>>(
     [collapsedGroups, onToggleGroup, renderGroupHeader, t],
   );
 
-  return {plugin, data: flattened, idKey};
+  return {plugin, data: flattened, idKey, isGroupHeader};
 }

packages/core/src/Table/useTableGroupedRows.doc.mjs

@@ -7,7 +7,7 @@ export const docs = {
   subComponentOf: 'Table',
   displayName: 'useTableGroupedRows',
   description:
-    'Hook that groups a flat data array into collapsible section rows. Each distinct groupBy value becomes a full-width section-header row with a chevron toggle, the group label, and a member count; collapsing hides that group\'s data rows while keeping the header visible. Mirrors useTableTreeState: the consumer owns the collapsedGroups set and the hook returns {data, plugin, idKey}: pass all three to Table (data, plugins, and idKey respectively).',
+    "Hook that groups a flat data array into collapsible section rows. Each distinct groupBy value becomes a full-width section-header row with a chevron toggle, the group label, and a member count; collapsing hides that group's data rows while keeping the header visible. Mirrors useTableTreeState: the consumer owns the collapsedGroups set and the hook returns {data, plugin, idKey, isGroupHeader}: pass the first three to Table (data, plugins, and idKey respectively), and use isGroupHeader to guard any row-level plugin or handler of your own that would otherwise treat a synthetic header as a real row.",
   props: [
     {
       name: 'data',
@@ -58,13 +58,14 @@ export const docs = {
 /** @type {import('@astryxdesign/cli/authoring').ComponentTranslationDoc} */
 export const docsDense = {
   description:
-    'Groups a flat data array into collapsible section rows. Each groupBy value becomes a full-width header (chevron + label + count); collapsing hides its rows. Returns {data, plugin, idKey}: pass to Table data / plugins / idKey. Consumer owns the collapsedGroups set.',
+    'Groups a flat data array into collapsible section rows. Each groupBy value becomes a full-width header (chevron + label + count); collapsing hides its rows. Returns {data, plugin, idKey, isGroupHeader}: pass the first three to Table data / plugins / idKey; use isGroupHeader to guard your own row-level plugins. Consumer owns the collapsedGroups set.',
   propDescriptions: {
     data: 'The flat data to group.',
-    groupBy: 'Derive a row\'s group key. Same key = same section.',
+    groupBy: "Derive a row's group key. Same key = same section.",
     collapsedGroups: 'Set of currently-collapsed group keys.',
     onToggleGroup: 'Called with the group key when a header is toggled.',
-    renderGroupHeader: "Custom header content (right of chevron). Default '<key> (<count>)'.",
+    renderGroupHeader:
+      "Custom header content (right of chevron). Default '<key> (<count>)'.",
     getRowKey: 'Stable key for a real row; positional fallback when omitted.',
     groupOrder: 'Pin these group keys first; others keep first-seen order.',
   },