feat: add sortEmpty option for comprehensive empty value handling #6086
Conversation
- Add sortEmpty option to handle null, undefined, and empty strings - Add isEmptyValue for custom empty value detection - Maintain backward compatibility with sortUndefined - Add deprecation warning for sortUndefined - Include comprehensive tests and example
WalkthroughAdds a new React example demonstrating Excel-like sorting with empty-value handling, introduces sortEmpty and isEmptyValue options in table-core sorting, maintains backward compatibility with sortUndefined (with deprecation warning), updates sorting flow to consider empty values, and adds comprehensive tests for new behaviors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as React App
participant Table as useReactTable
participant Sort as getSortedRowModel
participant Col as Column Def
User->>App: Click table header to sort
App->>Table: Update sorting state
Table->>Sort: Build sorted row model
Sort->>Col: Read sortEmpty / isEmptyValue / sortUndefined
alt sortEmpty set ('first'/'last')
Note right of Sort: Empty-aware path
Sort->>Sort: Compare A vs B emptiness via isEmptyValue
alt One empty
Sort-->>Table: Position per 'first'/'last'
else Both empty
Sort->>Sort: Defer to next criterion
else Neither empty
Sort->>Sort: Use sortingFn/invertSorting
end
else sortEmpty not set
Note right of Sort: Legacy path
Sort->>Sort: Apply sortUndefined handling
Sort->>Sort: Otherwise use sortingFn
end
Note over Col: Dev mode<br/>warn if sortUndefined used<br/>without sortEmpty
Table-->>App: Sorted rows
App-->>User: Rendered table with order
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (14)
examples/react/excel-sorting/tsconfig.node.json (1)
2-8: Add Node types for config tooling.Vite config runs in a Node context; include Node typings to avoid missing globals/types when editing vite.config.ts.
"compilerOptions": { "composite": true, "skipLibCheck": true, "module": "ESNext", "moduleResolution": "bundler", - "allowSyntheticDefaultImports": true + "allowSyntheticDefaultImports": true, + "types": ["node"] },examples/react/excel-sorting/vite.config.ts (1)
6-14: Allow serving monorepo sources outside example root (Vite fs allow).Aliases point outside the example; add fs.allow to avoid dev-server access errors in some setups.
export default defineConfig({ plugins: [react()], resolve: { alias: { '@tanstack/react-table': resolve(dirname, '../../../packages/react-table/src'), '@tanstack/table-core': resolve(dirname, '../../../packages/table-core/src'), }, }, + server: { + fs: { + allow: [ + resolve(dirname, '../../../packages'), + ], + }, + }, })examples/react/excel-sorting/tsconfig.json (1)
2-22: Include Vite client types and tighten casing checks.Helps with import.meta/env typing and avoids case-sensitivity bugs on CI/other OSes.
"compilerOptions": { "target": "ES2020", "useDefineForClassFields": true, "lib": ["ES2020", "DOM", "DOM.Iterable"], "module": "ESNext", "skipLibCheck": true, @@ "isolatedModules": true, "noEmit": true, "jsx": "react-jsx", + "types": ["vite/client"], @@ "strict": true, "noUnusedLocals": true, "noUnusedParameters": true, - "noFallthroughCasesInSwitch": true + "noFallthroughCasesInSwitch": true, + "forceConsistentCasingInFileNames": true },examples/react/excel-sorting/package.json (1)
1-10: Declare Node engine to match Vite 5 requirements.Prevents accidental use of unsupported Node versions.
{ "name": "tanstack-table-example-excel-sorting", "version": "0.0.0", "private": true, + "engines": { + "node": ">=18.0.0" + }, "scripts": { "dev": "vite", "build": "vite build", "serve": "vite preview", "start": "vite"examples/react/excel-sorting/src/index.css (1)
67-73: Border radius may not render with collapsed table borders.With border-collapse: collapse, border-radius often doesn’t clip. Consider separate + spacing 0, or move rounding to a wrapper.
-table { +table { width: 100%; - border-collapse: collapse; + border-collapse: separate; + border-spacing: 0; border: 1px solid #e0e0e0; border-radius: 6px; overflow: hidden; }packages/table-core/src/features/RowSorting.ts (2)
83-94: Clarify deprecation docs and wordingReplace “next column filter” with “next sort criterion” for accuracy.
* - `false` - * - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) + * - Undefined values will be considered tied and need to be sorted by the next sort criterion or original index (whichever applies)
95-115: Document whenisEmptyValueis appliedNote that
isEmptyValueis only used whensortEmptyis set to'first'or'last'./** - * Custom function to determine if a value should be considered empty + * Custom function to determine if a value should be considered empty. + * Only applied when `sortEmpty` is `'first'` or `'last'`.examples/react/excel-sorting/src/main.tsx (1)
199-224: Improve accessibility: expose current sort viaaria-sortAdd
aria-sortreflecting the column’s state; keep keyboard support as-is.- <th + <th key={header.id} style={{ width: header.getSize() }} onClick={header.column.getToggleSortingHandler()} onKeyDown={e => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() header.column.getToggleSortingHandler()?.(e) } }} tabIndex={header.column.getCanSort() ? 0 : -1} className={header.column.getCanSort() ? 'sortable' : ''} + aria-sort={ + header.column.getIsSorted() + ? (header.column.getIsSorted() === 'asc' + ? 'ascending' + : 'descending') + : 'none' + } >packages/table-core/src/utils/getSortedRowModel.ts (2)
25-34: MakeisEmptyValuenon-optional in internal mapYou always provide a fallback; drop the non-null assertion later.
- { + { sortUndefined?: false | -1 | 1 | 'first' | 'last' - sortEmpty?: false | 'first' | 'last' - isEmptyValue?: (value: unknown) => boolean + sortEmpty?: false | 'first' | 'last' + isEmptyValue: (value: unknown) => boolean invertSorting?: boolean sortingFn: SortingFn<TData> }
59-61: Minor micro-optimization: read cell values once per comparator iterationAvoid duplicate
getValuecalls across branches.- const sortEmpty = columnInfo.sortEmpty - const isEmptyValue = columnInfo.isEmptyValue! + const sortEmpty = columnInfo.sortEmpty + const isEmptyValue = columnInfo.isEmptyValue + const aValue = rowA.getValue(sortEntry.id) + const bValue = rowB.getValue(sortEntry.id)And then reuse
aValue/bValuewithin thesortEmptyandsortUndefinedblocks.packages/table-core/tests/sortEmpty.test.ts (4)
61-84: Tighten custom isEmptyValue to avoid treating 0/false as empty.The predicate uses "!value", which would classify 0/false as empty if reused elsewhere. For strings-only, trim is sufficient.
- isEmptyValue: (value) => !value || (typeof value === 'string' && value.trim() === ''), + isEmptyValue: (value) => + value == null || + (typeof value === 'string' && value.trim().length === 0),
26-41: Optional: Factor table creation helper to DRY up tests.Repeated createTable boilerplate can be reduced to a small helper for clarity and easier future changes.
Example helper (add near top of file):
function makeTable<TData>({ data, columns, sorting, }: { data: TData[] columns: any[] sorting: { id: string; desc?: boolean }[] }) { return createTable({ data, columns, getCoreRowModel: getCoreRowModel(), getSortedRowModel: getSortedRowModel(), state: { sorting }, onStateChange: () => {}, renderFallbackValue: null, }) }Then replace call sites with makeTable({ data: testData, columns, sorting: [...] }).
74-84: Add tie-break test when both values are empty to validate “continue to next sort” logic.This ensures that when both values are empty, the next sort column is used (or stable order applies).
it('should respect custom isEmptyValue function', () => { @@ }) + + it('uses next sort column when both values are empty', () => { + const table = createTable({ + data: testData, + columns, + getCoreRowModel: getCoreRowModel(), + getSortedRowModel: getSortedRowModel(), + state: { + sorting: [ + { id: 'price', desc: false }, // empties last + { id: 'name', desc: false }, // tie-breaker among empties + ], + }, + onStateChange: () => {}, + renderFallbackValue: null, + }) + const names = table.getSortedRowModel().rows.map(r => r.original.name) + // At the bottom, 'Product B' (null) should come before 'Product D' (undefined) by name + expect(names.slice(-2)).toEqual(['Product B', 'Product D']) + })
229-252: Cover 0 explicitly to prevent regressions (0 is not empty).Strengthen the default isEmptyValue test to assert that numeric 0 is treated as non-empty.
it('should use default isEmptyValue function (null, undefined, empty string)', () => { - const dataWithEmptyString = [ + const dataWithEmptyString = [ ...testData, { id: 6, name: 'Product F', price: '' as unknown as number, description: 'Description F' }, + { id: 7, name: 'Product G', price: 0, description: 'Description G' }, ] @@ - expect(prices).toEqual([50, 100, 200, null, undefined, '']) + expect(prices).toEqual([0, 50, 100, 200, null, undefined, ''])If the default sortingFn places 0 before other numbers differently, relax to:
expect(prices).toContain(0) const emptyStart = prices.findIndex(v => v == null || v === '') expect(prices.indexOf(0)).toBeGreaterThan(-1) expect(prices.indexOf(0)).toBeLessThan(emptyStart)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
examples/react/excel-sorting/index.html(1 hunks)examples/react/excel-sorting/package.json(1 hunks)examples/react/excel-sorting/src/index.css(1 hunks)examples/react/excel-sorting/src/main.tsx(1 hunks)examples/react/excel-sorting/tsconfig.json(1 hunks)examples/react/excel-sorting/tsconfig.node.json(1 hunks)examples/react/excel-sorting/vite.config.ts(1 hunks)packages/table-core/src/features/RowSorting.ts(3 hunks)packages/table-core/src/utils/getSortedRowModel.ts(3 hunks)packages/table-core/tests/sortEmpty.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/table-core/tests/sortEmpty.test.ts (2)
packages/table-core/src/columnHelper.ts (1)
createColumnHelper(73-91)packages/table-core/src/utils/getSortedRowModel.ts (1)
getSortedRowModel(5-144)
examples/react/excel-sorting/src/main.tsx (4)
packages/table-core/src/columnHelper.ts (1)
createColumnHelper(73-91)packages/react-table/src/index.tsx (1)
useReactTable(57-94)packages/table-core/src/utils/getCoreRowModel.ts (1)
getCoreRowModel(5-82)packages/table-core/src/utils/getSortedRowModel.ts (1)
getSortedRowModel(5-144)
packages/table-core/src/features/RowSorting.ts (1)
babel.config.cjs (1)
process(1-1)
🔇 Additional comments (5)
examples/react/excel-sorting/index.html (1)
1-12: LGTMEntry point looks good for the example.
packages/table-core/src/utils/getSortedRowModel.ts (2)
40-47: InitializeisEmptyValuewith guaranteed fallbackLooks good; with the above type tweak, you can remove
!at usage sites.
65-81: Confirm intended “pinned” behavior for empties
sortEmpty: 'first'|'last'returns early and ignoresdesc, so empties stay first/last regardless of direction. This matches the example’s “always bottom” claim; confirm this is the desired cross-library contract.packages/table-core/tests/sortEmpty.test.ts (2)
1-25: LGTM on test scaffolding and dataset.Type definitions, dataset variety (numbers, null, undefined, empty/whitespace strings), and use of createColumnHelper align well with the new API surface.
86-104: Empty‐value sorting isn’t inverted under desc
The comparator returns immediately on empty checks (lines 77–79) before applyingisDesc(lines 107–110), so empties stay in the absolute ‘first’/‘last’ position as your tests expect.Likely an incorrect or invalid review comment.
| import { | ||
| createColumnHelper, | ||
| flexRender, | ||
| getCoreRowModel, | ||
| getSortedRowModel, | ||
| useReactTable, | ||
| } from '@tanstack/react-table' | ||
| import './index.css' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type sorting state to SortingState
Prevents any[] inference and ensures proper type-checking with onSortingChange.
import {
createColumnHelper,
flexRender,
getCoreRowModel,
getSortedRowModel,
useReactTable,
+ type SortingState,
} from '@tanstack/react-table'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { | |
| createColumnHelper, | |
| flexRender, | |
| getCoreRowModel, | |
| getSortedRowModel, | |
| useReactTable, | |
| } from '@tanstack/react-table' | |
| import './index.css' | |
| import { | |
| createColumnHelper, | |
| flexRender, | |
| getCoreRowModel, | |
| getSortedRowModel, | |
| useReactTable, | |
| type SortingState, | |
| } from '@tanstack/react-table' | |
| import './index.css' |
🤖 Prompt for AI Agents
In examples/react/excel-sorting/src/main.tsx around lines 3 to 10, the local
sorting state is inferred as any[]; explicitly type it as SortingState to avoid
any[] inference and enable proper typing for onSortingChange. Import the
SortingState type from '@tanstack/react-table' (or reference it from the
existing imports), then declare the sorting state with
useState<SortingState>(initialValue) and ensure onSortingChange uses the setter
typed accordingly so the compiler enforces the correct sorting tuple structure.
| stock: number | null | ||
| category: string |
There was a problem hiding this comment.
Type mismatch: stock can be undefined in sample data
Data includes stock: undefined (Lines 50, 99). Expand the type to include undefined.
- stock: number | null
+ stock: number | null | undefined📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| stock: number | null | |
| category: string | |
| stock: number | null | undefined | |
| category: string |
🤖 Prompt for AI Agents
In examples/react/excel-sorting/src/main.tsx around lines 16-17, the stock field
is currently typed as "number | null" but sample data includes "stock:
undefined"; update the type to "number | null | undefined" (or "number |
undefined" if null isn't used elsewhere) and adjust any code that assumes stock
is always present (add optional chaining or null/undefined checks) so accesses
and renders handle undefined safely.
| function App() { | ||
| const [data] = React.useState(() => [...defaultData]) | ||
| const [sorting, setSorting] = React.useState([]) | ||
|
|
||
| const table = useReactTable({ | ||
| data, | ||
| columns, | ||
| state: { | ||
| sorting, | ||
| }, | ||
| onSortingChange: setSorting, | ||
| getCoreRowModel: getCoreRowModel(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialize sorting with explicit type
Aligns with table-core’s SortingState and avoids implicit any.
- const [sorting, setSorting] = React.useState([])
+ const [sorting, setSorting] = React.useState<SortingState>([])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function App() { | |
| const [data] = React.useState(() => [...defaultData]) | |
| const [sorting, setSorting] = React.useState([]) | |
| const table = useReactTable({ | |
| data, | |
| columns, | |
| state: { | |
| sorting, | |
| }, | |
| onSortingChange: setSorting, | |
| getCoreRowModel: getCoreRowModel(), | |
| function App() { | |
| const [data] = React.useState(() => [...defaultData]) | |
| const [sorting, setSorting] = React.useState<SortingState>([]) | |
| const table = useReactTable({ | |
| data, | |
| columns, | |
| state: { | |
| sorting, | |
| }, | |
| onSortingChange: setSorting, | |
| getCoreRowModel: getCoreRowModel(), |
🤖 Prompt for AI Agents
In examples/react/excel-sorting/src/main.tsx around lines 150 to 161, the
sorting state is created without an explicit type which leads to an implicit
any; import the table-core type (e.g. add import type { SortingState } from
'@tanstack/react-table') and change the state initialization to use that type
(e.g. React.useState<SortingState>([])) so sorting and setSorting are strongly
typed and align with the table core API.
| import { defineConfig } from 'vite' | ||
| import react from '@vitejs/plugin-react' | ||
| import { resolve } from 'path' | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix __dirname usage under ESM Vite config (runtime error).
This TS config is ESM; __dirname is undefined. Use fileURLToPath + URL and node: path/url imports.
-import { defineConfig } from 'vite'
-import react from '@vitejs/plugin-react'
-import { resolve } from 'path'
+import { defineConfig } from 'vite'
+import react from '@vitejs/plugin-react'
+import { resolve } from 'node:path'
+import { fileURLToPath } from 'node:url'
+
+const dirname = fileURLToPath(new URL('.', import.meta.url))
@@
- '@tanstack/react-table': resolve(__dirname, '../../../packages/react-table/src'),
- '@tanstack/table-core': resolve(__dirname, '../../../packages/table-core/src'),
+ '@tanstack/react-table': resolve(dirname, '../../../packages/react-table/src'),
+ '@tanstack/table-core': resolve(dirname, '../../../packages/table-core/src'),Run to confirm no stray __dirname remain:
Also applies to: 10-12
🏁 Script executed:
rg -n '\b__dirname\b' examples/react/excel-sorting/vite.config.tsLength of output: 247
Replace __dirname with ESM-compatible dirname in vite.config.ts
Lines 1–4: change
import { resolve } from 'path'to
import { resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
const dirname = fileURLToPath(new URL('.', import.meta.url))Lines 10–11: update
- '@tanstack/react-table': resolve(__dirname, '../../../packages/react-table/src'),
- '@tanstack/table-core': resolve(__dirname, '../../../packages/table-core/src'),
+ '@tanstack/react-table': resolve(dirname, '../../../packages/react-table/src'),
+ '@tanstack/table-core': resolve(dirname, '../../../packages/table-core/src'),🤖 Prompt for AI Agents
In examples/react/excel-sorting/vite.config.ts around lines 1–4 and 10–11,
replace the CommonJS __dirname usage and path import with ESM-compatible
equivalents: change the import of 'path' to import from 'node:path', add an
import of fileURLToPath from 'node:url', and define dirname using
fileURLToPath(new URL('.', import.meta.url)); then update any later references
on lines ~10–11 that used __dirname to use the new dirname constant so path
resolution works under ESM.
| if (column.columnDef.sortUndefined && !column.columnDef.sortEmpty) { | ||
| if (process.env.NODE_ENV !== 'production') { | ||
| console.warn( | ||
| `[TanStack Table] sortUndefined is deprecated. Please use sortEmpty instead for more comprehensive empty value handling. Column: ${column.id}` | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix false-positive deprecation warning when sortEmpty: false is used
The current truthy check warns even if users explicitly set sortEmpty: false. Warn only when sortUndefined is present AND sortEmpty is not provided (i.e., undefined).
- if (column.columnDef.sortUndefined && !column.columnDef.sortEmpty) {
+ if (
+ column.columnDef.sortUndefined !== undefined &&
+ column.columnDef.sortEmpty === undefined
+ ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (column.columnDef.sortUndefined && !column.columnDef.sortEmpty) { | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.warn( | |
| `[TanStack Table] sortUndefined is deprecated. Please use sortEmpty instead for more comprehensive empty value handling. Column: ${column.id}` | |
| ) | |
| } | |
| } | |
| if ( | |
| column.columnDef.sortUndefined !== undefined && | |
| column.columnDef.sortEmpty === undefined | |
| ) { | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.warn( | |
| `[TanStack Table] sortUndefined is deprecated. Please use sortEmpty instead for more comprehensive empty value handling. Column: ${column.id}` | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/table-core/src/features/RowSorting.ts around lines 329 to 335, the
deprecation warning currently triggers when sortUndefined is present and
sortEmpty is falsy, which incorrectly warns when users explicitly set sortEmpty:
false; change the condition to warn only when column.columnDef.sortUndefined is
truthy AND column.columnDef.sortEmpty is strictly undefined (e.g. typeof
column.columnDef.sortEmpty === 'undefined'), so explicit false values do not
trigger the warning; keep the NODE_ENV production guard and the same warning
message.
A new sortEmpty option has been introduced to handle all types of empty values, including null, undefined, and empty strings.
The previous sortUndefined option was limited to undefined values and could not properly handle other forms of empty data commonly found in real-world scenarios.
Therefore, sortUndefined is deprecated, and the use of sortEmpty is recommended as a more comprehensive solution.
Key Changes
Summary by CodeRabbit
New Features
Tests