feat(headless): add useDataTable hook#8868
Open
alexcarpenter wants to merge 6 commits into
Open
Conversation
Adds a server-driven data table state hook to @clerk/headless with sorting, column/global filtering, pagination, and row selection. Data passes through unchanged — consumers wire onChange callbacks to their own fetch layer.
🦋 Changeset detectedLatest commit: 0738664 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new ChangesuseDataTable Hook
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/headless/src/hooks/use-data-table.test.ts (1)
100-370: ⚡ Quick winAdd controlled-mode tests for the other controllable slices.
sortinghas a controlled-state test, butcolumnFilters,globalFilter,pagination, androwSelectiondo not. Since controlled/uncontrolled behavior is a core hook capability, add parity tests for those slices to prevent contract drift.As per coding guidelines, "
**/*.{test,spec}.{ts,tsx}: Unit tests are required for all new functionality."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/headless/src/hooks/use-data-table.test.ts` around lines 100 - 370, Add controlled-mode tests for the columnFilters, globalFilter, pagination, and rowSelection slices to establish parity with the existing controlled-state test for sorting. For each slice, create a test that verifies when the slice value is provided as a prop (e.g., columnFilters={[...]}, globalFilter="value", pagination={...}, rowSelection={...}), the hook respects that externally-managed state and only fires the corresponding callback (onColumnFiltersChange, onGlobalFilterChange, onPaginationChange, onRowSelectionChange) without internally overriding the value. This ensures the hook properly supports both controlled and uncontrolled patterns for all controllable slices.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/hip-ghosts-notice.md:
- Around line 1-2: The changeset file hip-ghosts-notice.md is empty and needs to
be populated with proper release metadata and changelog information. Add the
appropriate package bump type (major, minor, or patch) for `@clerk/headless` and
include a concise changelog summary describing the new public API being
introduced in this PR. Follow the standard changeset format to ensure the
feature includes proper version management and release notes when shipped.
In `@packages/headless/src/hooks/index.ts`:
- Around line 2-13: Remove the re-exports of useDataTable function and related
types (ColumnFiltersState, DataTableRow, OnChangeFn, PaginationState,
RowSelectionState, SortingState, Updater, UseDataTableOptions,
UseDataTableReturn) from the barrel index.ts file. Instead, consumers should
import these directly from the './use-data-table' module to avoid expanding the
barrel and reduce circular-dependency risk as per the repo guidelines.
In `@packages/headless/src/hooks/use-data-table.test.ts`:
- Around line 192-252: The existing pagination tests verify that getCanNextPage
and getCanPreviousPage return false at boundaries, but they do not test what
happens when nextPage or previousPage are actually called at those boundaries.
Add two new test cases: one that calls nextPage() when already on the last page
(totalCount equals data length multiplied by pageSize) and asserts that
pageIndex remains unchanged, and another that calls previousPage() when on page
0 and asserts that pageIndex remains 0. These tests will verify that the
navigation methods properly guard against out-of-range pageIndex values.
In `@packages/headless/src/hooks/use-data-table.ts`:
- Around line 6-78: Add comprehensive JSDoc comments to all public API exports
in this file to meet the documentation requirements for public-facing APIs.
Specifically, document the type definitions (Updater, OnChangeFn, SortingState,
ColumnFiltersState, PaginationState, RowSelectionState), interfaces
(UseDataTableOptions, DataTableRow, UseDataTableReturn), and the useDataTable
function. Each JSDoc should include clear descriptions of parameters, return
types, and behavior where applicable to support generated documentation.
- Line 184: The getIsSomeRowsSelected function on line 184 incorrectly returns
true whenever any rows are selected, including when all rows are selected. To
properly support indeterminate checkbox state, this function should only return
true for partial selection (some but not all rows selected). Modify the logic to
check that the number of selected rows is greater than zero AND less than the
total number of rows available in the rows array, ensuring it returns false when
either no rows are selected or all rows are selected.
- Around line 142-149: The row ID generation in the opts.data.map function uses
the array index (i) to create unstable selection keys that change when data is
sorted or paginated on the server. Replace the index-based id derivation with a
stable identifier from the original data object (such as a unique ID property
that persists across pages and sorts). This ensures that row selection remains
correct when the server returns data in a different order or on a different
page.
- Around line 168-174: The previousPage and setPageSize callbacks in
use-data-table.ts need input validation to prevent invalid pagination state. In
the previousPage callback, ensure pageIndex never goes below 0 by clamping the
decremented value. In the setPageSize callback, validate that the size parameter
is a positive number greater than 0 before updating pagination state. These
validations will prevent Line 157 from producing invalid Infinity or negative
counts due to invalid pageIndex or pageSize values.
---
Nitpick comments:
In `@packages/headless/src/hooks/use-data-table.test.ts`:
- Around line 100-370: Add controlled-mode tests for the columnFilters,
globalFilter, pagination, and rowSelection slices to establish parity with the
existing controlled-state test for sorting. For each slice, create a test that
verifies when the slice value is provided as a prop (e.g.,
columnFilters={[...]}, globalFilter="value", pagination={...},
rowSelection={...}), the hook respects that externally-managed state and only
fires the corresponding callback (onColumnFiltersChange, onGlobalFilterChange,
onPaginationChange, onRowSelectionChange) without internally overriding the
value. This ensures the hook properly supports both controlled and uncontrolled
patterns for all controllable slices.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: bb6dcb2b-627d-4ce4-8135-3e92de0c8ff5
📒 Files selected for processing (4)
.changeset/hip-ghosts-notice.mdpackages/headless/src/hooks/index.tspackages/headless/src/hooks/use-data-table.test.tspackages/headless/src/hooks/use-data-table.ts
- add getRowId option for stable row IDs across server-driven pages/sorts - clamp nextPage to last page, previousPage to 0, guard setPageSize > 0 - fix getIsSomeRowsSelected to return false when all rows selected - add boundary tests for nextPage/previousPage at limits - add controlled-mode tests for columnFilters, globalFilter, pagination, rowSelection
- pass onChange callbacks directly (avoids non-null assertions, OnChangeFn<T> is assignable to (value: T) => void via contravariance) - destructure getRowId before useMemo to satisfy react-hooks/exhaustive-deps - combine type and value imports from use-data-table to fix simple-import-sort
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
useDataTablehook to@clerk/headlessfor managing server-driven data table stateonChangecallbacks to their own fetch layeruseControllableStateSortingState,ColumnFiltersState,PaginationState) andUpdater<T>functional-update protocolUsage
Test plan
pnpm test --filter @clerk/headlesspassesSummary by CodeRabbit
useDataTablehook for data table state management, including sorting, column/global filtering, pagination, and row selection with controllable or default state.useDataTableto improve editor support and integration.useDataTabledocumentation to the hooks docs and Storybook, including options, return values, and key types.useDataTabletest suite covering row enrichment/stable IDs, controlled updates for sorting, filters, pagination, and row selection.