Skip to content

Commit 5ede300

Browse files
authored
fix: column order performance improvement (#6260)
1 parent db0c2d9 commit 5ede300

5 files changed

Lines changed: 268 additions & 25 deletions

File tree

packages/table-core/src/features/column-ordering/columnOrderingFeature.utils.ts

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -151,24 +151,27 @@ export function table_getOrderColumnsFn<
151151
if (!columnOrder?.length) {
152152
orderedColumns = columns
153153
} else {
154-
const columnOrderCopy = [...columnOrder]
155-
156-
// If there is an order, make a copy of the columns
157-
const columnsCopy = [...columns]
158-
159-
// And make a new ordered array of the columns
160-
161-
// Loop over the columns and place them in order into the new array
162-
while (columnsCopy.length && columnOrderCopy.length) {
163-
const targetColumnId = columnOrderCopy.shift()
164-
const foundIndex = columnsCopy.findIndex((d) => d.id === targetColumnId)
165-
if (foundIndex > -1) {
166-
orderedColumns.push(columnsCopy.splice(foundIndex, 1)[0]!)
154+
// Index columns by id for O(1) lookup
155+
const remaining = new Map<
156+
string,
157+
Column_Internal<TFeatures, TData, unknown>
158+
>()
159+
for (const column of columns) remaining.set(column.id, column)
160+
161+
// Place columns in the requested order, removing each as it's used
162+
// (handles duplicates and unknown ids in columnOrder)
163+
for (const id of columnOrder) {
164+
const column = remaining.get(id)
165+
if (column) {
166+
orderedColumns.push(column)
167+
remaining.delete(id)
167168
}
168169
}
169170

170-
// If there are any columns left, add them to the end
171-
orderedColumns = [...orderedColumns, ...columnsCopy]
171+
// Append leftover columns in their original order
172+
for (const column of columns) {
173+
if (remaining.has(column.id)) orderedColumns.push(column)
174+
}
172175
}
173176

174177
return orderColumns(table, orderedColumns)
@@ -207,9 +210,17 @@ export function orderColumns<
207210
return nonGroupingColumns
208211
}
209212

210-
const groupingColumns = grouping
211-
.map((g) => leafColumns.find((col) => col.id === g)!)
212-
.filter(Boolean)
213+
const leafColumnsById = new Map<
214+
string,
215+
Column_Internal<TFeatures, TData, unknown>
216+
>()
217+
for (const col of leafColumns) leafColumnsById.set(col.id, col)
218+
219+
const groupingColumns: Array<Column_Internal<TFeatures, TData, unknown>> = []
220+
for (const g of grouping) {
221+
const col = leafColumnsById.get(g)
222+
if (col) groupingColumns.push(col)
223+
}
213224

214225
return [...groupingColumns, ...nonGroupingColumns]
215226
}

packages/table-core/src/features/row-sorting/rowSortingFeature.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ export function constructRowSortingFeature<
8282

8383
assignColumnPrototype(prototype, table) {
8484
assignPrototypeAPIs('rowSortingFeature', prototype, table, {
85-
'column.getAutoSortFn': {
85+
column_getAutoSortFn: {
8686
fn: (column) => column_getAutoSortFn(column),
8787
},
88-
'column.getAutoSortDir': {
88+
column_getAutoSortDir: {
8989
fn: (column) => column_getAutoSortDir(column),
9090
},
9191
column_getSortFn: {

packages/table-core/src/fns/sortFns.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,14 @@ function compareAlphanumeric(aStr: string, bStr: string) {
157157
const a = aStr.split(reSplitAlphaNumeric).filter(Boolean)
158158
const b = bStr.split(reSplitAlphaNumeric).filter(Boolean)
159159

160-
// While
161-
while (a.length && b.length) {
162-
const aa = a.shift()!
163-
const bb = b.shift()!
160+
let ai = 0
161+
let bi = 0
162+
const aLen = a.length
163+
const bLen = b.length
164+
165+
while (ai < aLen && bi < bLen) {
166+
const aa = a[ai++]!
167+
const bb = b[bi++]!
164168

165169
const an = parseInt(aa, 10)
166170
const bn = parseInt(bb, 10)
@@ -192,7 +196,7 @@ function compareAlphanumeric(aStr: string, bStr: string) {
192196
}
193197
}
194198

195-
return a.length - b.length
199+
return aLen - ai - (bLen - bi)
196200
}
197201

198202
// Exports

packages/table-core/tests/unit/features/column-ordering/columnOrderingFeature.utils.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,57 @@ describe('table_getOrderColumnsFn', () => {
133133
expect(orderedColumns[0]?.id).toBe('lastName')
134134
expect(orderedColumns[1]?.id).toBe('firstName')
135135
})
136+
137+
it('should append leftover columns in original order when columnOrder is partial', () => {
138+
const table = generateTestTableWithData<TableFeatures>(3, {
139+
initialState: {
140+
columnOrder: ['age', 'firstName'],
141+
},
142+
})
143+
const columns = table.getAllLeafColumns()
144+
const originalIds = columns.map((c: any) => c.id)
145+
const orderFn = table_getOrderColumnsFn(table)
146+
const orderedIds = orderFn(columns).map((c: any) => c.id)
147+
148+
expect(orderedIds.slice(0, 2)).toEqual(['age', 'firstName'])
149+
const leftoverIds = originalIds.filter(
150+
(id: string) => id !== 'age' && id !== 'firstName',
151+
)
152+
expect(orderedIds.slice(2)).toEqual(leftoverIds)
153+
})
154+
155+
it('should skip unknown ids in columnOrder', () => {
156+
const table = generateTestTableWithData<TableFeatures>(3, {
157+
initialState: {
158+
columnOrder: ['unknown1', 'lastName', 'unknown2', 'firstName'],
159+
},
160+
})
161+
const columns = table.getAllLeafColumns()
162+
const originalIds = columns.map((c: any) => c.id)
163+
const orderFn = table_getOrderColumnsFn(table)
164+
const orderedIds = orderFn(columns).map((c: any) => c.id)
165+
166+
expect(orderedIds.slice(0, 2)).toEqual(['lastName', 'firstName'])
167+
expect(orderedIds).toHaveLength(originalIds.length)
168+
expect(new Set(orderedIds)).toEqual(new Set(originalIds))
169+
})
170+
171+
it('should not duplicate columns when columnOrder contains duplicates', () => {
172+
const table = generateTestTableWithData<TableFeatures>(3, {
173+
initialState: {
174+
columnOrder: ['lastName', 'lastName', 'firstName'],
175+
},
176+
})
177+
const columns = table.getAllLeafColumns()
178+
const originalIds = columns.map((c: any) => c.id)
179+
const orderFn = table_getOrderColumnsFn(table)
180+
const orderedIds = orderFn(columns).map((c: any) => c.id)
181+
182+
expect(orderedIds).toHaveLength(originalIds.length)
183+
expect(new Set(orderedIds)).toEqual(new Set(originalIds))
184+
expect(orderedIds[0]).toBe('lastName')
185+
expect(orderedIds[1]).toBe('firstName')
186+
})
136187
})
137188

138189
describe('orderColumns', () => {
@@ -168,4 +219,22 @@ describe('orderColumns', () => {
168219

169220
expect(orderedColumns[0]?.id).toBe('lastName')
170221
})
222+
223+
it('should preserve grouping order and original order for non-grouping when reordering', () => {
224+
const table = generateTestTableWithData<TableFeatures>(3, {
225+
initialState: {
226+
grouping: ['age', 'firstName'],
227+
},
228+
groupedColumnMode: 'reorder',
229+
})
230+
const columns = table.getAllLeafColumns()
231+
const originalIds = columns.map((c: any) => c.id)
232+
const orderedIds = orderColumns(table, columns).map((c: any) => c.id)
233+
234+
expect(orderedIds.slice(0, 2)).toEqual(['age', 'firstName'])
235+
const leftoverIds = originalIds.filter(
236+
(id: string) => id !== 'age' && id !== 'firstName',
237+
)
238+
expect(orderedIds.slice(2)).toEqual(leftoverIds)
239+
})
171240
})
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
import { describe, expect, it } from 'vitest'
2+
import {
3+
sortFn_alphanumeric,
4+
sortFn_alphanumericCaseSensitive,
5+
sortFn_basic,
6+
sortFn_datetime,
7+
sortFn_text,
8+
sortFn_textCaseSensitive,
9+
} from '../../../src'
10+
11+
const makeRow = (value: any): any => ({
12+
getValue: () => value,
13+
})
14+
15+
const cmp = (
16+
fn: (a: any, b: any, c: string) => number,
17+
a: any,
18+
b: any,
19+
): number => {
20+
const result = fn(makeRow(a), makeRow(b), 'col')
21+
// Normalize to -1 / 0 / 1 for stable assertions across sign-only comparators
22+
return result < 0 ? -1 : result > 0 ? 1 : 0
23+
}
24+
25+
describe('sortFn_alphanumeric (compareAlphanumeric)', () => {
26+
it('returns 0 for equal strings', () => {
27+
expect(cmp(sortFn_alphanumeric, 'abc', 'abc')).toBe(0)
28+
})
29+
30+
it('returns 0 for two empty strings', () => {
31+
expect(cmp(sortFn_alphanumeric, '', '')).toBe(0)
32+
})
33+
34+
it('sorts pure strings lexicographically', () => {
35+
expect(cmp(sortFn_alphanumeric, 'apple', 'banana')).toBe(-1)
36+
expect(cmp(sortFn_alphanumeric, 'banana', 'apple')).toBe(1)
37+
})
38+
39+
it('sorts pure numbers numerically (natural sort)', () => {
40+
// The whole point: "item2" should come before "item10"
41+
expect(cmp(sortFn_alphanumeric, 'item2', 'item10')).toBe(-1)
42+
expect(cmp(sortFn_alphanumeric, 'item10', 'item2')).toBe(1)
43+
})
44+
45+
it('sorts mixed alphanumeric strings naturally', () => {
46+
expect(cmp(sortFn_alphanumeric, 'a1b2', 'a1b10')).toBe(-1)
47+
expect(cmp(sortFn_alphanumeric, 'a2b', 'a10b')).toBe(-1)
48+
})
49+
50+
it('treats string chunk less than number chunk in mixed-type comparison', () => {
51+
// When chunk types differ at the same position, string sorts before number
52+
expect(cmp(sortFn_alphanumeric, 'abc', '123')).toBe(-1)
53+
expect(cmp(sortFn_alphanumeric, '123', 'abc')).toBe(1)
54+
})
55+
56+
it('returns difference in chunk count when one is a prefix of the other', () => {
57+
// After all common chunks are consumed, the longer one wins (positive)
58+
expect(cmp(sortFn_alphanumeric, 'abc', 'abc1')).toBe(-1)
59+
expect(cmp(sortFn_alphanumeric, 'abc1', 'abc')).toBe(1)
60+
})
61+
62+
it('returns 0 when both strings normalize to no chunks', () => {
63+
// Empty strings split+filter to []
64+
expect(cmp(sortFn_alphanumeric, '', '')).toBe(0)
65+
})
66+
67+
it('lowercases input (case-insensitive)', () => {
68+
expect(cmp(sortFn_alphanumeric, 'ABC', 'abc')).toBe(0)
69+
expect(cmp(sortFn_alphanumeric, 'Apple', 'banana')).toBe(-1)
70+
})
71+
72+
it('coerces numbers to strings via toString', () => {
73+
expect(cmp(sortFn_alphanumeric, 2, 10)).toBe(-1)
74+
expect(cmp(sortFn_alphanumeric, 10, 2)).toBe(1)
75+
})
76+
77+
it('treats NaN/Infinity numbers as empty', () => {
78+
expect(cmp(sortFn_alphanumeric, NaN, '')).toBe(0)
79+
expect(cmp(sortFn_alphanumeric, Infinity, '')).toBe(0)
80+
})
81+
82+
it('handles long mixed strings (stress: many chunks)', () => {
83+
const a = 'abc1def2ghi3jkl4mno5'
84+
const b = 'abc1def2ghi3jkl4mno10'
85+
expect(cmp(sortFn_alphanumeric, a, b)).toBe(-1)
86+
expect(cmp(sortFn_alphanumeric, b, a)).toBe(1)
87+
})
88+
89+
it('sort() integration: produces natural order', () => {
90+
const items = ['item10', 'item2', 'item1', 'item20', 'item3']
91+
items.sort((a, b) => sortFn_alphanumeric(makeRow(a), makeRow(b), 'col'))
92+
expect(items).toEqual(['item1', 'item2', 'item3', 'item10', 'item20'])
93+
})
94+
})
95+
96+
describe('sortFn_alphanumericCaseSensitive', () => {
97+
it('preserves case (uppercase < lowercase by ASCII)', () => {
98+
expect(cmp(sortFn_alphanumericCaseSensitive, 'ABC', 'abc')).toBe(-1)
99+
expect(cmp(sortFn_alphanumericCaseSensitive, 'abc', 'ABC')).toBe(1)
100+
})
101+
102+
it('sorts naturally with case sensitivity', () => {
103+
expect(cmp(sortFn_alphanumericCaseSensitive, 'Item2', 'Item10')).toBe(-1)
104+
})
105+
})
106+
107+
describe('sortFn_text', () => {
108+
it('returns 0 for equal strings', () => {
109+
expect(cmp(sortFn_text, 'abc', 'abc')).toBe(0)
110+
})
111+
112+
it('sorts lexicographically (no natural sort)', () => {
113+
// text uses compareBasic — "item10" < "item2" lexicographically
114+
expect(cmp(sortFn_text, 'item10', 'item2')).toBe(-1)
115+
})
116+
117+
it('lowercases input', () => {
118+
expect(cmp(sortFn_text, 'ABC', 'abc')).toBe(0)
119+
})
120+
})
121+
122+
describe('sortFn_textCaseSensitive', () => {
123+
it('preserves case', () => {
124+
expect(cmp(sortFn_textCaseSensitive, 'ABC', 'abc')).toBe(-1)
125+
})
126+
})
127+
128+
describe('sortFn_basic', () => {
129+
it('returns 0 for equal values', () => {
130+
expect(cmp(sortFn_basic, 5, 5)).toBe(0)
131+
})
132+
133+
it('compares numbers', () => {
134+
expect(cmp(sortFn_basic, 1, 2)).toBe(-1)
135+
expect(cmp(sortFn_basic, 2, 1)).toBe(1)
136+
})
137+
138+
it('compares strings', () => {
139+
expect(cmp(sortFn_basic, 'a', 'b')).toBe(-1)
140+
})
141+
})
142+
143+
describe('sortFn_datetime', () => {
144+
it('returns 0 for equal dates', () => {
145+
const d = new Date(2026, 1, 1)
146+
expect(cmp(sortFn_datetime, d, d)).toBe(0)
147+
})
148+
149+
it('compares dates by chronology', () => {
150+
const earlier = new Date(2026, 1, 1)
151+
const later = new Date(2026, 1, 2)
152+
expect(cmp(sortFn_datetime, earlier, later)).toBe(-1)
153+
expect(cmp(sortFn_datetime, later, earlier)).toBe(1)
154+
})
155+
156+
it('compares numeric timestamps', () => {
157+
expect(cmp(sortFn_datetime, 1000, 2000)).toBe(-1)
158+
})
159+
})

0 commit comments

Comments
 (0)