From 240ff91d85f516ee2eff27b3b6c63176790dcda7 Mon Sep 17 00:00:00 2001 From: Luke Cotter <4013877+lukecotter@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:11:14 +0100 Subject: [PATCH 1/2] perf(sort): precompute sort keys and fast-path numeric sorter _sortItems decorates each row with its sort keys and component once, sorts the decorated array, then writes rows back, instead of re-deriving values inside every comparison. The number sorter returns a-b directly for finite numbers, skipping String()/split. Adds Sort._sortItems and number-sorter unit tests. --- src/js/modules/Sort/Sort.js | 103 ++++++++++----- .../modules/Sort/defaults/sorters/number.js | 61 +++++---- test/unit/modules/SortItems.spec.js | 124 ++++++++++++++++++ test/unit/modules/SortNumberSorter.spec.js | 79 +++++++++++ 4 files changed, 310 insertions(+), 57 deletions(-) create mode 100644 test/unit/modules/SortItems.spec.js create mode 100644 test/unit/modules/SortNumberSorter.spec.js diff --git a/src/js/modules/Sort/Sort.js b/src/js/modules/Sort/Sort.js index 116046777..1814682f5 100644 --- a/src/js/modules/Sort/Sort.js +++ b/src/js/modules/Sort/Sort.js @@ -166,19 +166,20 @@ export default class Sort extends Module{ sorters=[], match = false; - if(column.modules.sort){ - if(column.modules.sort.tristate){ - if(column.modules.sort.dir == "none"){ - dir = column.modules.sort.startingDir; + const sortModule = column.modules.sort; + if(sortModule){ + if(sortModule.tristate){ + if(sortModule.dir == "none"){ + dir = sortModule.startingDir; }else{ - if(column.modules.sort.dir == column.modules.sort.startingDir){ - dir = column.modules.sort.dir == "asc" ? "desc" : "asc"; + if(sortModule.dir == sortModule.startingDir){ + dir = sortModule.dir == "asc" ? "desc" : "asc"; }else{ dir = "none"; } } }else{ - switch(column.modules.sort.dir){ + switch(sortModule.dir){ case "asc": dir = "desc"; break; @@ -188,7 +189,7 @@ export default class Sort extends Module{ break; default: - dir = column.modules.sort.startingDir; + dir = sortModule.startingDir; } } @@ -340,7 +341,7 @@ export default class Sort extends Module{ //work through sort list sorting data sort(data, sortOnly){ var self = this, - sortList = this.table.options.sortOrderReverse ? self.sortList.slice().reverse() : self.sortList, + sortList = this.table.options.sortOrderReverse ? self.sortList.toReversed(): self.sortList, sortListActual = [], rowComponents = []; @@ -355,32 +356,29 @@ export default class Sort extends Module{ if(this.table.options.sortMode !== "remote"){ //build list of valid sorters and trigger column specific callbacks before sort begins - sortList.forEach(function(item, i){ - var sortObj; - - if(item.column){ - sortObj = item.column.modules.sort; - + for(const item of sortList) { + const column = item.column; + if(column){ + const sortObj = column.modules.sort; if(sortObj){ - //if no sorter has been defined, take a guess if(!sortObj.sorter){ - sortObj.sorter = self.findSorter(item.column); + sortObj.sorter = self.findSorter(column); } - item.params = typeof sortObj.params === "function" ? sortObj.params(item.column.getComponent(), item.dir) : sortObj.params; + item.params = typeof sortObj.params === "function" ? sortObj.params(column.getComponent(), item.dir) : sortObj.params; sortListActual.push(item); } if(!sortOnly) { - self.setColumnHeader(item.column, item.dir); + self.setColumnHeader(column, item.dir); } } - }); + } //sort data - if (sortListActual.length) { + if (sortListActual.length && data.length) { self._sortItems(data, sortListActual); } @@ -439,23 +437,58 @@ export default class Sort extends Module{ //sort each item in sort list _sortItems(data, sortList){ - var sorterCount = sortList.length - 1; - - data.sort((a, b) => { - var result; - - for(var i = sorterCount; i>= 0; i--){ - let sortItem = sortList[i]; - - result = this._sortRow(a, b, sortItem.column, sortItem.dir, sortItem.params); - + const sortMeta = sortList.map((sortItem) => { + return { + column: sortItem.column, + dir: sortItem.dir, + params: sortItem.params, + sorter: sortItem.column.modules.sort.sorter, + columnComponent: sortItem.column.getComponent(), + asc: sortItem.dir === "asc", + }; + }); + + const sorterCount = sortMeta.length - 1; + const length = data.length; + + //extract each row's sort keys and component once, then sort the decorated array + const decorated = new Array(length); + for(let k = 0; k < length; k++){ + const row = data[k]; + const rowData = row.getData(); + const values = new Array(sortMeta.length); + for(let j = 0; j <= sorterCount; j++){ + const value = sortMeta[j].column.getFieldValue(rowData); + values[j] = typeof value !== "undefined" ? value : ""; + } + decorated[k] = {values, component: row.getComponent(), row}; + } + + decorated.sort((a, b) => { + for(let i = sorterCount; i >= 0; i--){ + const sortItem = sortMeta[i]; + const asc = sortItem.asc; + const result = sortItem.sorter.call(this, + asc ? a.values[i] : b.values[i], + asc ? b.values[i] : a.values[i], + asc ? a.component : b.component, + asc ? b.component : a.component, + sortItem.columnComponent, + sortItem.dir, + sortItem.params + ); + if(result !== 0){ - break; + return result; } } - - return result; + + return 0; }); + + for(let k = 0; k < length; k++){ + data[k] = decorated[k].row; + } } //process individual rows for a sort function on active data @@ -477,4 +510,4 @@ export default class Sort extends Module{ return column.modules.sort.sorter.call(this, a, b, el1Comp, el2Comp, column.getComponent(), dir, params); } -} \ No newline at end of file +} diff --git a/src/js/modules/Sort/defaults/sorters/number.js b/src/js/modules/Sort/defaults/sorters/number.js index d96dcd364..67a1846a2 100644 --- a/src/js/modules/Sort/defaults/sorters/number.js +++ b/src/js/modules/Sort/defaults/sorters/number.js @@ -4,31 +4,30 @@ export default function(a, b, aRow, bRow, column, dir, params){ var decimal = params.decimalSeparator; var thousand = params.thousandSeparator; var emptyAlign = 0; + var aEmpty = a === "" || a === null || typeof a === "undefined"; + var bEmpty = b === "" || b === null || typeof b === "undefined"; - a = String(a); - b = String(b); - - if(thousand){ - a = a.split(thousand).join(""); - b = b.split(thousand).join(""); - } - - if(decimal){ - a = a.split(decimal).join("."); - b = b.split(decimal).join("."); + if(typeof a === "number" && typeof b === "number" && isFinite(a) && isFinite(b)){ + return a - b; } - a = parseFloat(a); - b = parseFloat(b); - - //handle non numeric values - if(isNaN(a)){ - emptyAlign = isNaN(b) ? 0 : -1; - }else if(isNaN(b)){ - emptyAlign = 1; + if(aEmpty){ + emptyAlign = bEmpty ? 0 : -1; + }else if(bEmpty){ + emptyAlign = 1; }else{ - //compare valid values - return a - b; + a = parseValue(a, decimal, thousand); + b = parseValue(b, decimal, thousand); + + //handle non numeric values + if(isNaN(a)){ + emptyAlign = isNaN(b) ? 0 : -1; + }else if(isNaN(b)){ + emptyAlign = 1; + }else{ + //compare valid values + return a - b; + } } //fix empty values in position @@ -37,4 +36,22 @@ export default function(a, b, aRow, bRow, column, dir, params){ } return emptyAlign; -} \ No newline at end of file +} + +function parseValue(value, decimal, thousand){ + if(typeof value === "number"){ + return value; + } + + value = String(value); + + if(thousand){ + value = value.replaceAll(thousand, ""); + } + + if(decimal && decimal !== "." ){ + value = value.replaceAll(decimal, "."); + } + + return parseFloat(value); +} diff --git a/test/unit/modules/SortItems.spec.js b/test/unit/modules/SortItems.spec.js new file mode 100644 index 000000000..3b3d6ad3a --- /dev/null +++ b/test/unit/modules/SortItems.spec.js @@ -0,0 +1,124 @@ +import Sort from "../../../src/js/modules/Sort/Sort"; +import numberSorter from "../../../src/js/modules/Sort/defaults/sorters/number"; +import stringSorter from "../../../src/js/modules/Sort/defaults/sorters/string"; + +// Correctness coverage for Sort._sortItems decorate-sort-undecorate rewrite. +// Validates ordering + stability against a reference implementation of the stock +// per-comparison algorithm. Standalone (no TabulatorFull/luxon) so it runs in the +// unit environment. + +const ctx = {}; // sorter `this`; number/string sorters do not use it here + +const numParams = { alignEmptyValues: undefined, decimalSeparator: undefined, thousandSeparator: undefined }; +const strParams = { alignEmptyValues: undefined, locale: false }; + +function makeColumn(field, sorter, params) { + return { + field, + _comp: null, + getFieldValue(data) { return data[field]; }, + getComponent() { return (this._comp ||= { _col: field }); }, + modules: { sort: { sorter, params } }, + }; +} + +function makeRow(data) { + return { data, _comp: null, getData() { return this.data; }, getComponent() { return (this._comp ||= { _row: this.data }); } }; +} + +// Reference: the stock per-comparison algorithm (matches HEAD _sortItems/_sortRow). +function referenceSort(data, sortList) { + const sorterCount = sortList.length - 1; + return data.slice().sort((a, b) => { + let result; + for (let i = sorterCount; i >= 0; i--) { + const s = sortList[i]; + const el1 = s.dir === "asc" ? a : b; + const el2 = s.dir === "asc" ? b : a; + let av = s.column.getFieldValue(el1.getData()); + let bv = s.column.getFieldValue(el2.getData()); + av = typeof av !== "undefined" ? av : ""; + bv = typeof bv !== "undefined" ? bv : ""; + result = s.column.modules.sort.sorter.call(ctx, av, bv, el1.getComponent(), el2.getComponent(), s.column.getComponent(), s.dir, s.params); + if (result !== 0) break; + } + return result; + }); +} + +function runSort(data, sortList) { + const copy = data.slice(); + Sort.prototype._sortItems.call(ctx, copy, sortList); + return copy; +} + +describe("Sort._sortItems (decorate-sort-undecorate)", () => { + test("single numeric column ascending", () => { + const col = makeColumn("a", numberSorter, numParams); + const rows = [makeRow({ a: 3 }), makeRow({ a: 1 }), makeRow({ a: 2 })]; + const out = runSort(rows, [{ column: col, dir: "asc", params: numParams }]); + expect(out.map((r) => r.data.a)).toEqual([1, 2, 3]); + }); + + test("single numeric column descending", () => { + const col = makeColumn("a", numberSorter, numParams); + const rows = [makeRow({ a: 3 }), makeRow({ a: 1 }), makeRow({ a: 2 })]; + const out = runSort(rows, [{ column: col, dir: "desc", params: numParams }]); + expect(out.map((r) => r.data.a)).toEqual([3, 2, 1]); + }); + + test("mutates the array in place (same reference)", () => { + const col = makeColumn("a", numberSorter, numParams); + const rows = [makeRow({ a: 2 }), makeRow({ a: 1 })]; + const ref = rows; + Sort.prototype._sortItems.call(ctx, rows, [{ column: col, dir: "asc", params: numParams }]); + expect(rows).toBe(ref); + expect(rows.map((r) => r.data.a)).toEqual([1, 2]); + }); + + test("multi-column: primary (last in list) then tie-break", () => { + const colA = makeColumn("a", numberSorter, numParams); + const colB = makeColumn("b", numberSorter, numParams); + const rows = [ + makeRow({ a: 2, b: 1 }), makeRow({ a: 1, b: 2 }), makeRow({ a: 1, b: 1 }), makeRow({ a: 2, b: 2 }), + ]; + const sortList = [ + { column: colB, dir: "asc", params: numParams }, // tie-break (checked first in loop) + { column: colA, dir: "asc", params: numParams }, // primary (checked last) + ]; + const out = runSort(rows, sortList); + expect(out.map((r) => [r.data.a, r.data.b])).toEqual(referenceSort(rows, sortList).map((r) => [r.data.a, r.data.b])); + }); + + test("stability: equal keys preserve original order", () => { + const col = makeColumn("a", numberSorter, numParams); + const rows = [makeRow({ a: 1, id: "x" }), makeRow({ a: 1, id: "y" }), makeRow({ a: 1, id: "z" })]; + const out = runSort(rows, [{ column: col, dir: "asc", params: numParams }]); + expect(out.map((r) => r.data.id)).toEqual(["x", "y", "z"]); + }); + + test("matches reference over randomized multi-column (numeric + string, asc + desc)", () => { + const colA = makeColumn("a", numberSorter, numParams); + const colName = makeColumn("name", stringSorter, strParams); + const sortList = [ + { column: colName, dir: "desc", params: strParams }, + { column: colA, dir: "asc", params: numParams }, + ]; + let seed = 99; + const rnd = () => { seed = (seed * 1103515245 + 12345) & 0x7fffffff; return seed / 0x7fffffff; }; + for (let trial = 0; trial < 50; trial++) { + const rows = []; + for (let i = 0; i < 200; i++) rows.push(makeRow({ a: Math.floor(rnd() * 5), name: "n" + Math.floor(rnd() * 5), id: i })); + const expected = referenceSort(rows, sortList).map((r) => r.data.id); + const actual = runSort(rows, sortList).map((r) => r.data.id); + expect(actual).toEqual(expected); + } + }); + + test("undefined field values are coerced to empty string (as stock)", () => { + const col = makeColumn("a", numberSorter, numParams); + const rows = [makeRow({ a: 5 }), makeRow({}), makeRow({ a: 2 })]; + const out = runSort(rows, [{ column: col, dir: "asc", params: numParams }]); + expect(out.map((r) => r.data.a)).toEqual(referenceSort(rows, [{ column: col, dir: "asc", params: numParams }]).map((r) => r.data.a)); + }); +}); diff --git a/test/unit/modules/SortNumberSorter.spec.js b/test/unit/modules/SortNumberSorter.spec.js new file mode 100644 index 000000000..02b7cf571 --- /dev/null +++ b/test/unit/modules/SortNumberSorter.spec.js @@ -0,0 +1,79 @@ +import numberSorter from "../../../src/js/modules/Sort/defaults/sorters/number"; + +describe("number sorter", () => { + const asc = "asc"; + const desc = "desc"; + + const defaultParams = { + alignEmptyValues: undefined, + decimalSeparator: undefined, + thousandSeparator: undefined, + }; + + test("sorts plain numeric values", () => { + expect(numberSorter(2, 10, null, null, null, asc, defaultParams)).toBe(-8); + expect(numberSorter(10, 2, null, null, null, asc, defaultParams)).toBe(8); + expect(numberSorter(2, 2, null, null, null, asc, defaultParams)).toBe(0); + }); + + test("supports thousand separator normalization", () => { + const params = { + ...defaultParams, + thousandSeparator: ",", + }; + + expect(numberSorter("1,200", "900", null, null, null, asc, params)).toBe(300); + expect(numberSorter("900", "1,200", null, null, null, asc, params)).toBe(-300); + }); + + test("supports decimal separator normalization", () => { + const params = { + ...defaultParams, + decimalSeparator: ",", + }; + + expect(numberSorter("1,5", "1,25", null, null, null, asc, params)).toBe(0.25); + }); + + test("handles both values as non-numeric", () => { + expect(numberSorter("foo", "bar", null, null, null, asc, defaultParams)).toBe(0); + }); + + test("default empty alignment keeps non-numeric first in asc path", () => { + expect(numberSorter("foo", "1", null, null, null, asc, defaultParams)).toBe(-1); + expect(numberSorter("1", "foo", null, null, null, asc, defaultParams)).toBe(1); + }); + + test("treats empty primitive values as non-numeric", () => { + expect(numberSorter("", 1, null, null, null, asc, defaultParams)).toBe(-1); + expect(numberSorter(null, 1, null, null, null, asc, defaultParams)).toBe(-1); + expect(numberSorter(undefined, 1, null, null, null, asc, defaultParams)).toBe(-1); + }); + + test("handles mixed numeric primitives and numeric strings", () => { + const params = { + ...defaultParams, + thousandSeparator: ",", + }; + + expect(numberSorter(1200, "900", null, null, null, asc, params)).toBe(300); + expect(numberSorter("1,200", 900, null, null, null, asc, params)).toBe(300); + }); + + test("alignEmptyValues top and bottom flip as expected by direction", () => { + const topParams = { + ...defaultParams, + alignEmptyValues: "top", + }; + const bottomParams = { + ...defaultParams, + alignEmptyValues: "bottom", + }; + + expect(numberSorter("foo", "1", null, null, null, asc, topParams)).toBe(-1); + expect(numberSorter("foo", "1", null, null, null, desc, topParams)).toBe(1); + + expect(numberSorter("foo", "1", null, null, null, asc, bottomParams)).toBe(1); + expect(numberSorter("foo", "1", null, null, null, desc, bottomParams)).toBe(-1); + }); +}); From 5740ccc2566023b7db51cc35d0c8ae32b1b58256 Mon Sep 17 00:00:00 2001 From: Luke Cotter <4013877+lukecotter@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:21:31 +0100 Subject: [PATCH 2/2] refactor(sort): apply review follow-ups to sort perf work - rebuild SortItems tests on real TabulatorFull instances instead of hand-built row/column fakes, so they exercise the actual Row/Column contract - remove the dead _sortRow method orphaned by the _sortItems rewrite - revert sortList reversal to slice().reverse(); toReversed() is ES2023 and the lib ships no polyfills, so it would raise the implicit ~ES2021 baseline - hoist the numeric fast-path above empty/param setup in the number sorter --- src/js/modules/Sort/Sort.js | 22 +- .../modules/Sort/defaults/sorters/number.js | 9 +- test/unit/modules/SortItems.spec.js | 188 +++++++++--------- 3 files changed, 98 insertions(+), 121 deletions(-) diff --git a/src/js/modules/Sort/Sort.js b/src/js/modules/Sort/Sort.js index 1814682f5..d1ca9d023 100644 --- a/src/js/modules/Sort/Sort.js +++ b/src/js/modules/Sort/Sort.js @@ -341,7 +341,7 @@ export default class Sort extends Module{ //work through sort list sorting data sort(data, sortOnly){ var self = this, - sortList = this.table.options.sortOrderReverse ? self.sortList.toReversed(): self.sortList, + sortList = this.table.options.sortOrderReverse ? self.sortList.slice().reverse() : self.sortList, sortListActual = [], rowComponents = []; @@ -490,24 +490,4 @@ export default class Sort extends Module{ data[k] = decorated[k].row; } } - - //process individual rows for a sort function on active data - _sortRow(a, b, column, dir, params){ - var el1Comp, el2Comp; - - //switch elements depending on search direction - var el1 = dir == "asc" ? a : b; - var el2 = dir == "asc" ? b : a; - - a = column.getFieldValue(el1.getData()); - b = column.getFieldValue(el2.getData()); - - a = typeof a !== "undefined" ? a : ""; - b = typeof b !== "undefined" ? b : ""; - - el1Comp = el1.getComponent(); - el2Comp = el2.getComponent(); - - return column.modules.sort.sorter.call(this, a, b, el1Comp, el2Comp, column.getComponent(), dir, params); - } } diff --git a/src/js/modules/Sort/defaults/sorters/number.js b/src/js/modules/Sort/defaults/sorters/number.js index 67a1846a2..4ee4a7648 100644 --- a/src/js/modules/Sort/defaults/sorters/number.js +++ b/src/js/modules/Sort/defaults/sorters/number.js @@ -1,5 +1,10 @@ //sort numbers export default function(a, b, aRow, bRow, column, dir, params){ + //fast-path: both already finite numbers, no separator/empty handling needed + if(typeof a === "number" && typeof b === "number" && isFinite(a) && isFinite(b)){ + return a - b; + } + var alignEmptyValues = params.alignEmptyValues; var decimal = params.decimalSeparator; var thousand = params.thousandSeparator; @@ -7,10 +12,6 @@ export default function(a, b, aRow, bRow, column, dir, params){ var aEmpty = a === "" || a === null || typeof a === "undefined"; var bEmpty = b === "" || b === null || typeof b === "undefined"; - if(typeof a === "number" && typeof b === "number" && isFinite(a) && isFinite(b)){ - return a - b; - } - if(aEmpty){ emptyAlign = bEmpty ? 0 : -1; }else if(bEmpty){ diff --git a/test/unit/modules/SortItems.spec.js b/test/unit/modules/SortItems.spec.js index 3b3d6ad3a..8b1acc4ff 100644 --- a/test/unit/modules/SortItems.spec.js +++ b/test/unit/modules/SortItems.spec.js @@ -1,124 +1,120 @@ -import Sort from "../../../src/js/modules/Sort/Sort"; -import numberSorter from "../../../src/js/modules/Sort/defaults/sorters/number"; -import stringSorter from "../../../src/js/modules/Sort/defaults/sorters/string"; +import TabulatorFull from "../../../src/js/core/TabulatorFull"; // Correctness coverage for Sort._sortItems decorate-sort-undecorate rewrite. -// Validates ordering + stability against a reference implementation of the stock -// per-comparison algorithm. Standalone (no TabulatorFull/luxon) so it runs in the -// unit environment. +// Builds real rows/columns via TabulatorFull (the Sort.spec.js pattern) and +// validates ordering + stability against a reference implementation of the +// stock per-comparison algorithm, so the tests exercise the real Row/Column +// contract rather than hand-built fakes. -const ctx = {}; // sorter `this`; number/string sorters do not use it here - -const numParams = { alignEmptyValues: undefined, decimalSeparator: undefined, thousandSeparator: undefined }; -const strParams = { alignEmptyValues: undefined, locale: false }; +describe("Sort._sortItems (decorate-sort-undecorate)", () => { + /** @type {TabulatorFull} */ + let tabulator; + let sortMod; -function makeColumn(field, sorter, params) { - return { - field, - _comp: null, - getFieldValue(data) { return data[field]; }, - getComponent() { return (this._comp ||= { _col: field }); }, - modules: { sort: { sorter, params } }, - }; -} + const columns = [ + { title: "A", field: "a", sorter: "number" }, + { title: "B", field: "b", sorter: "number" }, + { title: "Name", field: "name", sorter: "string" }, + { title: "Id", field: "id", sorter: "number" }, + ]; -function makeRow(data) { - return { data, _comp: null, getData() { return this.data; }, getComponent() { return (this._comp ||= { _row: this.data }); } }; -} + beforeEach(() => { + const el = document.createElement("div"); + el.id = "tabulator"; + document.body.appendChild(el); + tabulator = new TabulatorFull("#tabulator", { data: [], columns }); + sortMod = tabulator.module("sort"); + return new Promise((resolve) => tabulator.on("tableBuilt", resolve)); + }); -// Reference: the stock per-comparison algorithm (matches HEAD _sortItems/_sortRow). -function referenceSort(data, sortList) { - const sorterCount = sortList.length - 1; - return data.slice().sort((a, b) => { - let result; - for (let i = sorterCount; i >= 0; i--) { - const s = sortList[i]; - const el1 = s.dir === "asc" ? a : b; - const el2 = s.dir === "asc" ? b : a; - let av = s.column.getFieldValue(el1.getData()); - let bv = s.column.getFieldValue(el2.getData()); - av = typeof av !== "undefined" ? av : ""; - bv = typeof bv !== "undefined" ? bv : ""; - result = s.column.modules.sort.sorter.call(ctx, av, bv, el1.getComponent(), el2.getComponent(), s.column.getComponent(), s.dir, s.params); - if (result !== 0) break; - } - return result; + afterEach(() => { + tabulator.destroy(); + document.getElementById("tabulator")?.remove(); }); -} -function runSort(data, sortList) { - const copy = data.slice(); - Sort.prototype._sortItems.call(ctx, copy, sortList); - return copy; -} + // Reference: an independent per-comparison oracle (the pre-rewrite sort algorithm), + // run over the same real rows/columns _sortItems sees. + function referenceSort(rows, sortList) { + const sorterCount = sortList.length - 1; + return rows.slice().sort((a, b) => { + let result = 0; + for (let i = sorterCount; i >= 0; i--) { + const { column, dir } = sortList[i]; + const { sorter, params } = column.modules.sort; + const el1 = dir === "asc" ? a : b; + const el2 = dir === "asc" ? b : a; + let av = column.getFieldValue(el1.getData()); + let bv = column.getFieldValue(el2.getData()); + av = typeof av !== "undefined" ? av : ""; + bv = typeof bv !== "undefined" ? bv : ""; + result = sorter.call(sortMod, av, bv, el1.getComponent(), el2.getComponent(), column.getComponent(), dir, params); + if (result !== 0) break; + } + return result; + }); + } -describe("Sort._sortItems (decorate-sort-undecorate)", () => { - test("single numeric column ascending", () => { - const col = makeColumn("a", numberSorter, numParams); - const rows = [makeRow({ a: 3 }), makeRow({ a: 1 }), makeRow({ a: 2 })]; - const out = runSort(rows, [{ column: col, dir: "asc", params: numParams }]); - expect(out.map((r) => r.data.a)).toEqual([1, 2, 3]); + // Load data, apply sort, and return { actual, expected, original } where actual is + // produced by Sort.sort() (which delegates to _sortItems) and expected by the reference. + async function sortData(data, sortSpec) { + await tabulator.setData(data); + const sortList = sortSpec.map(({ field, dir }) => ({ column: tabulator.columnManager.findColumn(field), dir })); + sortMod.setSort(sortList); + const original = tabulator.rowManager.activeRows.slice(); + const actual = sortMod.sort(original.slice()); + return { actual, expected: referenceSort(original, sortList) }; + } + + const ids = (rows) => rows.map((row) => row.data.id); + const field = (rows, key) => rows.map((row) => row.data[key]); + + test("single numeric column ascending", async () => { + const { actual } = await sortData([{ id: 1, a: 3 }, { id: 2, a: 1 }, { id: 3, a: 2 }], [{ field: "a", dir: "asc" }]); + expect(field(actual, "a")).toEqual([1, 2, 3]); }); - test("single numeric column descending", () => { - const col = makeColumn("a", numberSorter, numParams); - const rows = [makeRow({ a: 3 }), makeRow({ a: 1 }), makeRow({ a: 2 })]; - const out = runSort(rows, [{ column: col, dir: "desc", params: numParams }]); - expect(out.map((r) => r.data.a)).toEqual([3, 2, 1]); + test("single numeric column descending", async () => { + const { actual } = await sortData([{ id: 1, a: 3 }, { id: 2, a: 1 }, { id: 3, a: 2 }], [{ field: "a", dir: "desc" }]); + expect(field(actual, "a")).toEqual([3, 2, 1]); }); - test("mutates the array in place (same reference)", () => { - const col = makeColumn("a", numberSorter, numParams); - const rows = [makeRow({ a: 2 }), makeRow({ a: 1 })]; + test("sorts the passed data array in place (same reference)", async () => { + await tabulator.setData([{ id: 1, a: 2 }, { id: 2, a: 1 }]); + sortMod.setSort([{ column: tabulator.columnManager.findColumn("a"), dir: "asc" }]); + const rows = tabulator.rowManager.activeRows.slice(); const ref = rows; - Sort.prototype._sortItems.call(ctx, rows, [{ column: col, dir: "asc", params: numParams }]); - expect(rows).toBe(ref); - expect(rows.map((r) => r.data.a)).toEqual([1, 2]); + const out = sortMod.sort(rows); + expect(out).toBe(ref); + expect(field(rows, "a")).toEqual([1, 2]); }); - test("multi-column: primary (last in list) then tie-break", () => { - const colA = makeColumn("a", numberSorter, numParams); - const colB = makeColumn("b", numberSorter, numParams); - const rows = [ - makeRow({ a: 2, b: 1 }), makeRow({ a: 1, b: 2 }), makeRow({ a: 1, b: 1 }), makeRow({ a: 2, b: 2 }), + test("multi-column ordering matches reference", async () => { + const data = [ + { id: 1, a: 2, b: 1 }, { id: 2, a: 1, b: 2 }, { id: 3, a: 1, b: 1 }, { id: 4, a: 2, b: 2 }, ]; - const sortList = [ - { column: colB, dir: "asc", params: numParams }, // tie-break (checked first in loop) - { column: colA, dir: "asc", params: numParams }, // primary (checked last) - ]; - const out = runSort(rows, sortList); - expect(out.map((r) => [r.data.a, r.data.b])).toEqual(referenceSort(rows, sortList).map((r) => [r.data.a, r.data.b])); + const { actual, expected } = await sortData(data, [{ field: "b", dir: "asc" }, { field: "a", dir: "asc" }]); + expect(ids(actual)).toEqual(ids(expected)); }); - test("stability: equal keys preserve original order", () => { - const col = makeColumn("a", numberSorter, numParams); - const rows = [makeRow({ a: 1, id: "x" }), makeRow({ a: 1, id: "y" }), makeRow({ a: 1, id: "z" })]; - const out = runSort(rows, [{ column: col, dir: "asc", params: numParams }]); - expect(out.map((r) => r.data.id)).toEqual(["x", "y", "z"]); + test("stability: equal keys preserve original order", async () => { + const { actual } = await sortData([{ id: 1, a: 1 }, { id: 2, a: 1 }, { id: 3, a: 1 }], [{ field: "a", dir: "asc" }]); + expect(ids(actual)).toEqual([1, 2, 3]); }); - test("matches reference over randomized multi-column (numeric + string, asc + desc)", () => { - const colA = makeColumn("a", numberSorter, numParams); - const colName = makeColumn("name", stringSorter, strParams); - const sortList = [ - { column: colName, dir: "desc", params: strParams }, - { column: colA, dir: "asc", params: numParams }, - ]; + test("matches reference over randomized multi-column (numeric + string, asc + desc)", async () => { + const sortSpec = [{ field: "name", dir: "desc" }, { field: "a", dir: "asc" }]; let seed = 99; const rnd = () => { seed = (seed * 1103515245 + 12345) & 0x7fffffff; return seed / 0x7fffffff; }; - for (let trial = 0; trial < 50; trial++) { - const rows = []; - for (let i = 0; i < 200; i++) rows.push(makeRow({ a: Math.floor(rnd() * 5), name: "n" + Math.floor(rnd() * 5), id: i })); - const expected = referenceSort(rows, sortList).map((r) => r.data.id); - const actual = runSort(rows, sortList).map((r) => r.data.id); - expect(actual).toEqual(expected); + for (let trial = 0; trial < 10; trial++) { + const data = []; + for (let i = 0; i < 60; i++) data.push({ id: i, a: Math.floor(rnd() * 5), name: "n" + Math.floor(rnd() * 5) }); + const { actual, expected } = await sortData(data, sortSpec); + expect(ids(actual)).toEqual(ids(expected)); } }); - test("undefined field values are coerced to empty string (as stock)", () => { - const col = makeColumn("a", numberSorter, numParams); - const rows = [makeRow({ a: 5 }), makeRow({}), makeRow({ a: 2 })]; - const out = runSort(rows, [{ column: col, dir: "asc", params: numParams }]); - expect(out.map((r) => r.data.a)).toEqual(referenceSort(rows, [{ column: col, dir: "asc", params: numParams }]).map((r) => r.data.a)); + test("undefined field values are coerced to empty string (as stock)", async () => { + const { actual, expected } = await sortData([{ id: 1, a: 5 }, { id: 2 }, { id: 3, a: 2 }], [{ field: "a", dir: "asc" }]); + expect(ids(actual)).toEqual(ids(expected)); }); });