Skip to content

TreeDataGrid: type group keys and IDs as strings, and export GroupRow - #4263

Merged
nstepien merged 4 commits into
mainfrom
breaking/tree-string-group-keys
Oct 29, 2026
Merged

nstepien merged 4 commits into
mainfrom
breaking/tree-string-group-keys

Conversation

@nstepien

@nstepien nstepien commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Since #4159, getRowGroupKey and getGroupId return strings, so TreeDataGrid only ever produces string group keys and IDs, but the types still said unknown. Apps had to cast groupKey as string, or keep expandedGroupIds in a ReadonlySet<unknown> that could hold values the grid never produces. This types them all as string, and exports GroupRow, which already appeared in public types (RenderGroupCellContentProps.row, RowHeightArgs) but couldn't be named.

This is the first PR of a stack of four: #4264 builds on it, and #4272 renames GroupRow to RowGroup.

Breaking change

- const [expandedGroupIds, setExpandedGroupIds] = useState((): ReadonlySet<unknown> => new Set());
+ const [expandedGroupIds, setExpandedGroupIds] = useState((): ReadonlySet<string> => new Set());

expandedGroupIds, onExpandedGroupIdsChange, GroupRow.groupKey, GroupRow.parentId, and RenderGroupCellContentProps.groupKey are typed with string instead of unknown, which is what getRowGroupKey and getGroupId produce. GroupRow is now exported.

Changes

  • GroupRow.parentId is string | undefined (undefined on the first level) and GroupRow.groupKey is string; every GroupRow member now has JSDoc, checked against expandGroups and setStartRowIndexes.
  • RenderGroupCellContentProps.groupKey is string; RenderGroupCellContentProps: pass the group row only #4264 then removes it in favor of row.groupKey.
  • The internal toggleGroup parameters of TreeDataGrid, GroupedRow, and GroupCell are string, and renderToggleGroup no longer casts groupKey as string.
  • GroupRow is exported from src/index.ts; its README entry is no longer "(internal)" and sits next to RenderGroupCellContentProps, and the RenderGroupCellContentProps and RowHeightArgs entries link to it.
  • The ReadonlySet<unknown> state in the row grouping demo and in TreeDataGrid.test.tsx is now ReadonlySet<string>. The new Set() literals passed to expandedGroupIds infer Set<string> and need no change.

Verification

vp run ci passes. No tests are added, because the existing code already type-checks the new types: setup() in TreeDataGrid.test.tsx and the row grouping demo pass ReadonlySet<string> state and its setter to the grid, toggleGroup passes new Set(expandedGroupIds) to onExpandedGroupIdsChange, and renderToggleGroup renders groupKey without a cast. A consumer type-check of the built lib/index.d.ts, with and without exactOptionalPropertyTypes, confirms that ReadonlySet<unknown> state is a type error at expandedGroupIds and that ReadonlySet<string> state accepts the Set<string> from onExpandedGroupIdsChange.

🤖 Generated with Claude Code

`getRowGroupKey` and `getGroupId` return strings, so `expandedGroupIds`,
`onExpandedGroupIdsChange`, `GroupRow.groupKey`, `GroupRow.parentId`, and
`RenderGroupCellContentProps.groupKey` are now typed with `string` instead
of `unknown`. `GroupRow` is exported and its members are documented.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nstepien
nstepien added this pull request to stack #4266 October 27, 2026 19:50
@nstepien nstepien changed the title TreeDataGrid: type group keys and IDs as strings, and export GroupRow TreeDataGrid: type group keys and IDs as strings, and export GroupRow Oct 27, 2026
@nstepien nstepien changed the title TreeDataGrid: type group keys and IDs as strings, and export GroupRow TreeDataGrid: type group keys and IDs as strings, and export GroupRow Oct 27, 2026
@codecov-commenter

codecov-commenter commented Oct 27, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.27%. Comparing base (0f4c62c) to head (b32633a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4263   +/-   ##
=======================================
  Coverage   99.27%   99.27%           
=======================================
  Files          44       44           
  Lines        2216     2216           
  Branches      974      974           
=======================================
  Hits         2200     2200           
  Misses         16       16           
Files with missing lines Coverage Δ
src/GroupCell.tsx 100.00% <ø> (ø)
src/GroupedRow.tsx 100.00% <ø> (ø)
src/TreeDataGrid.tsx 98.66% <100.00%> (ø)
src/cellRenderers/renderToggleGroup.tsx 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nstepien
nstepien marked this pull request as ready for review October 28, 2026 11:02
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nstepien and others added 2 commits October 29, 2026 02:05
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its assertions restated the declarations. `setup()` and the row grouping demo already pass `ReadonlySet<string>` state and its setter to the grid, `toggleGroup` passes `new Set(expandedGroupIds)` to `onExpandedGroupIdsChange`, and `renderToggleGroup` renders `groupKey` without a cast.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nstepien
nstepien merged commit 38417f2 into main Oct 29, 2026
5 checks passed
@nstepien
nstepien deleted the breaking/tree-string-group-keys branch October 29, 2026 02:11
nstepien added a commit that referenced this pull request Oct 29, 2026
`RenderGroupCellContentProps` had `groupKey`, `childRows`, and
`isExpanded` next to `row`, a `GroupRow` that carries the same three
properties. The duplication dates from when `GroupRow` couldn't be named
outside the package; now that it's exported, the props are `{ column,
row, tabIndex, toggleGroup }`, which leaves one shape to document and a
smaller object per group cell.

Builds on #4263, which exports `GroupRow` and types its keys as strings.

### Breaking change

```diff
- renderGroupCell({ childRows }) {
-   return childRows.reduce((sum, row) => sum + row.gold, 0);
+ renderGroupCell({ row }) {
+   return row.childRows.reduce((sum, childRow) => sum + childRow.gold, 0);
  }
```

`groupKey`, `childRows`, and `isExpanded` are removed from
`RenderGroupCellContentProps`; read them from `row`, which is a
`GroupRow`.

### Changes

- `GroupCell`'s own props are slimmed to `{ column, row, isCellActive,
isGroupByColumn, isLevelMatching, toggleGroup }`, and it reads `row.id`
for the toggle. `row` was already one of its props, so its memoization
invalidates exactly as before.
- `ToggleGroup` reads `row.groupKey` and `row.isExpanded`.
- The row grouping demo, `TreeDataGrid.test.tsx`, and
`test/visual/treeGrid.test.tsx` (type-checked only) read
`row.childRows`.
- README: the `RenderGroupCellContentProps` block matches the new JSDoc,
`renderGroupCell` gets an example, and `renderToggleGroup` says it
renders `row.groupKey`.

### Verification

Static checks (`vp check`, `vp run typecheck`, `vp run eslint`) and `vp
pack` pass, and `TreeDataGrid.test.tsx` and `columnOrder.test.tsx` pass
in Chromium, Firefox, and WebKit. The `custom renderGroupCell` test
covers the `row.childRows` path. No type test is added, because
`ToggleGroup`, the row grouping demo, and that test already read the
group from `row`. `vp run ci` passes on the top of the stack (#4272).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants