From a24045cb4240a5557e8d741763c1b1ed917a91d6 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Sun, 30 Aug 2020 14:38:12 +0200 Subject: [PATCH] Refactor sorting to return a JSONPatchDocument --- package-lock.json | 10 +- package.json | 2 +- public/index.html | 2 + src/components/modals/SortModal.svelte | 19 ++-- src/components/treemode/TreeMode.svelte | 18 ++-- src/logic/sort.js | 128 ++++++++++++++-------- src/logic/sort.test.js | 134 ++++++++++++++++++------ 7 files changed, 208 insertions(+), 105 deletions(-) diff --git a/package-lock.json b/package-lock.json index c28a4d2..2358803 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1351,11 +1351,6 @@ "iterate-iterator": "^1.0.1" } }, - "javascript-natural-sort": { - "version": "0.7.1", - "resolved": "https://registry.npmjs.org/javascript-natural-sort/-/javascript-natural-sort-0.7.1.tgz", - "integrity": "sha1-+eIwPUUH9tdDVac2ZNFED7Wg71k=" - }, "jest-worker": { "version": "26.0.0", "resolved": "https://registry.npmjs.org/jest-worker/-/jest-worker-26.0.0.tgz", @@ -1635,6 +1630,11 @@ "integrity": "sha1-Sr6/7tdUHywnrPspvbvRXI1bpPc=", "dev": true }, + "natural-compare-lite": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/natural-compare-lite/-/natural-compare-lite-1.4.0.tgz", + "integrity": "sha1-F7CVgZiJef3a/gIB6TG6kzyWy7Q=" + }, "normalize-package-data": { "version": "2.5.0", "resolved": "https://registry.npmjs.org/normalize-package-data/-/normalize-package-data-2.5.0.tgz", diff --git a/package.json b/package.json index 133a81b..a9befd5 100644 --- a/package.json +++ b/package.json @@ -21,8 +21,8 @@ "ace-builds": "1.4.12", "ajv": "6.12.3", "classnames": "2.2.6", - "javascript-natural-sort": "0.7.1", "lodash-es": "4.17.15", + "natural-compare-lite": "1.4.0", "svelte-awesome": "2.3.0", "svelte-select": "3.11.1", "svelte-simple-modal": "0.6.0" diff --git a/public/index.html b/public/index.html index 85703bf..e844ad9 100644 --- a/public/index.html +++ b/public/index.html @@ -120,12 +120,14 @@ console.time('create large json') const largeJson = {} largeJson.numbers = [] + largeJson.randomNumbers = [] largeJson.array = [] for (let i = 0; i < count; i++) { const longitude = 4 + i / count const latitude = 51 + i / count largeJson.numbers.push(i) + largeJson.randomNumbers.push(Math.round(Math.random() * 1000)) largeJson.array.push({ name: 'Item ' + i, id: String(i), diff --git a/src/components/modals/SortModal.svelte b/src/components/modals/SortModal.svelte index 4cd6e5d..9b9f797 100644 --- a/src/components/modals/SortModal.svelte +++ b/src/components/modals/SortModal.svelte @@ -10,7 +10,7 @@ import { sortArray, sortObjectKeys } from '../../logic/sort.js' export let json - export let path + export let rootPath export let onSort const {close} = getContext('simple-modal') @@ -57,17 +57,14 @@ const property = selectedProperty.value const direction = selectedDirection.value - const sortedJson = sortArray(json, property, direction) + const operations = sortArray(json, rootPath, property, direction) - onSort(sortedJson) + onSort(operations) } else if (isObject(json)) { const direction = selectedDirection.value - const sortedJson = sortObjectKeys(json, direction) - - // FIXME: the keys are now sorted, but the JSONEditor refuses to reorder when already rendered -> need to do a JSONPatch - console.log('sorted object keys:', Object.keys(sortedJson)) - - onSort(sortedJson) + const operations = sortObjectKeys(json, rootPath, direction) + + onSort(operations) } else { console.error('Cannot sort: no array or object') } @@ -86,7 +83,7 @@ - {#if path.length > 0} + {#if rootPath.length > 0} Path @@ -94,7 +91,7 @@ class="path" type="text" readonly - value={stringifyPath(path)} + value={stringifyPath(rootPath)} /> diff --git a/src/components/treemode/TreeMode.svelte b/src/components/treemode/TreeMode.svelte index bb87154..9fc4acd 100644 --- a/src/components/treemode/TreeMode.svelte +++ b/src/components/treemode/TreeMode.svelte @@ -263,29 +263,23 @@ } function handleSort () { - const path = selection && selection.paths + const rootPath = selection && selection.paths ? selection.paths.length > 1 ? initial(first(selection.paths)) // the parent path of the paths : first(selection.paths) // the first and only path : [] open(SortModal, { - json: getIn(doc, path), - path, - onSort: async sortedJson => { - console.log('onSort', path, sortedJson) + json: getIn(doc, rootPath), + rootPath, + onSort: async (operations) => { + console.log('onSort', rootPath, operations) - // TODO: replace this with move events instead of a big replace (currently we lose state) - const operations = [{ - op: 'replace', - path: compileJSONPointer(path), - value: sortedJson - }] patch(operations, selection) await tick() - handleExpand(path, true, false) + handleExpand(rootPath, true, false) } }, { ...SIMPLE_MODAL_OPTIONS, diff --git a/src/logic/sort.js b/src/logic/sort.js index ceb7e03..44981dd 100644 --- a/src/logic/sort.js +++ b/src/logic/sort.js @@ -1,17 +1,80 @@ +import naturalCompare from 'natural-compare-lite' import { getIn } from '../utils/immutabilityHelpers.js' -import naturalSort from 'javascript-natural-sort' +import { compileJSONPointer } from '../utils/jsonPointer.js' + +function caseInsensitiveNaturalCompare (a, b) { + const aLower = typeof a === 'string' ? a.toLowerCase() : a + const bLower = typeof b === 'string' ? b.toLowerCase() : b + + return naturalCompare(aLower, bLower) +} + +/** + * Sort the keys of an object + * @param {Object} object The object to be sorted + * @param {Path} [rootPath=[]] Relative path when the array was located + * @param {1 | -1} [direction=1] Pass 1 to sort ascending, -1 to sort descending + * @return {JSONPatchDocument} Returns a JSONPatch document with move operation + * to get the array sorted. + */ +export function sortObjectKeys (object, rootPath = [], direction = 1) { + const keys = Object.keys(object) + const sortedKeys = keys.slice() + + sortedKeys.sort((keyA, keyB) => { + return direction * caseInsensitiveNaturalCompare(keyA, keyB) + }) + + // const sortedObject = {} + // keys.forEach(key => { + // sortedObject[key] = object[key] + // }) + + // TODO: only move the properties that are needed to move + const operations = [] + for (let i = 0; i < sortedKeys.length; i++) { + const key = sortedKeys[i] + const path = compileJSONPointer(rootPath.concat(key)) + operations.push({ + op: 'move', + from: path, + path + }) + } + + return operations +} /** * Sort the items of an array - * @param {Array} array The array to be sorted - * @param {Path} [path=[]] Nested path to the property on which to sort the contents - * @param {1 | -1} [direction=1] Pass 1 to sort ascending, -1 to sort descending - * @return {Array} Returns a sorted shallow copy of the array + * @param {Array} array The array to be sorted + * @param {Path} [rootPath=[]] Relative path when the array was located + * @param {Path} [propertyPath=[]] Nested path to the property on which to sort the contents + * @param {1 | -1} [direction=1] Pass 1 to sort ascending, -1 to sort descending + * @return {JSONPatchDocument} Returns a JSONPatch document with move operation + * to get the array sorted. */ -export function sortArray (array, path = [], direction = 1) { - function comparator (a, b) { - const valueA = getIn(a, path) - const valueB = getIn(b, path) +export function sortArray (array, rootPath = [], propertyPath = [], direction = 1) { + const comparator = createObjectComparator(propertyPath, direction) + + return getSortingMoves(array, comparator).map(({ fromIndex, toIndex }) => { + return { + op: 'move', + from: compileJSONPointer(rootPath.concat(fromIndex)), + path: compileJSONPointer(rootPath.concat(toIndex)) + } + }) +} + +/** + * Create a comparator function to compare nested properties in an array + * @param {Path} propertyPath + * @param {1 | -1} direction + */ +function createObjectComparator (propertyPath, direction) { + return function comparator (a, b) { + const valueA = getIn(a, propertyPath) + const valueB = getIn(b, propertyPath) if (valueA === undefined) { return direction @@ -22,54 +85,31 @@ export function sortArray (array, path = [], direction = 1) { if (typeof valueA !== 'string' && typeof valueB !== 'string') { // both values are a number, boolean, or null -> use simple, fast sorting - return valueA > valueB - ? direction - : valueA < valueB - ? -direction + return valueA > valueB + ? direction + : valueA < valueB + ? -direction : 0 } - return direction * naturalSort(valueA, valueB) + return direction * caseInsensitiveNaturalCompare(valueA, valueB) } - - // TODO: use lodash orderBy, split comparator and direction? - const sortedArray = array.slice() - sortedArray.sort(comparator) - - return sortedArray -} - -/** - * Sort the keys of an object - * @param {Object} object The object to be sorted - * @param {1 | -1} [direction=1] Pass 1 to sort ascending, -1 to sort descending - * @return {Object} Returns a sorted shallow copy of the object - */ -export function sortObjectKeys (object, direction = 1) { - const keys = Object.keys(object) - keys.sort((keyA, keyB) => { - return direction * naturalSort(keyA, keyB) - }) - - const sortedObject = {} - keys.forEach(key => sortedObject[key] = object[key]) - - return sortedObject } /** * Create an array containing all move operations * needed to sort the array contents. - * @param {Array} array - * @param {function (a, b) => number} comparator - * @param {Array.<{fromIndex: number, toIndex: number}>} + * @param {Array} array + * @param {function (a, b) => number} comparator + * @param {Array.<{fromIndex: number, toIndex: number}>} */ -export function sortMoveOperations (array, comparator) { +export function getSortingMoves (array, comparator) { const operations = [] const sorted = [] + // TODO: rewrite the function to pass a callback instead of returning an array? for (let i = 0; i < array.length; i++) { - // TODO: implement a faster way to sort (binary tree sort?) + // TODO: implement a faster way to sort. Something with longest increasing subsequence? // TODO: can we simplify the following code? const item = array[i] if (i > 0 && comparator(sorted[i - 1], item) > 0) { @@ -83,7 +123,7 @@ export function sortMoveOperations (array, comparator) { toIndex: j }) - sorted.splice(j, 0, [item]) + sorted.splice(j, 0, item) } else { sorted.push(item) } diff --git a/src/logic/sort.test.js b/src/logic/sort.test.js index e76556c..4e0abea 100644 --- a/src/logic/sort.test.js +++ b/src/logic/sort.test.js @@ -1,61 +1,131 @@ import assert from 'assert' -import { sortArray, sortObjectKeys, sortMoveOperations } from './sort.js' +import { sortArray, sortObjectKeys, getSortingMoves } from './sort.js' -describe.only('sort', () => { - - it('should sort array', () => { - assert.deepStrictEqual(sortArray([ 2, 3, 1 ]), [1, 2, 3]) - assert.deepStrictEqual(sortArray([ 2, 3, 1 ], undefined, -1), [3, 2, 1]) - }) - - it('should sort array using natural sort', () => { - assert.deepStrictEqual(sortArray([ '10', '2', '1' ]), ['1', '2', '10']) - }) - - it('should sort array by nested properties', () => { - const a = {data: { value: 1 }} - const b = {data: { value: 2 }} - const c = {data: { value: 3 }} - - assert.deepStrictEqual(sortArray([ b, c, a ], ['data', 'value']), [a, b, c]) - assert.deepStrictEqual(sortArray([ b, a, c ], ['data', 'value']), [a, b, c]) - assert.deepStrictEqual(sortArray([ b, a, c ], ['data', 'value'], 1), [a, b, c]) - assert.deepStrictEqual(sortArray([ b, a, c ], ['data', 'value'], -1), [c, b, a]) - }) - +describe('sort', () => { it('should sort object keys', () => { const object = { b: 1, c: 1, a: 1 } - assert.deepStrictEqual(Object.keys(sortObjectKeys(object)), ['a', 'b', 'c']) - assert.deepStrictEqual(Object.keys(sortObjectKeys(object, 1)), ['a', 'b', 'c']) - assert.deepStrictEqual(Object.keys(sortObjectKeys(object, -1)), ['c', 'b', 'a']) + assert.deepStrictEqual(sortObjectKeys(object), [ + { op: 'move', from: '/a', path: '/a' }, + { op: 'move', from: '/b', path: '/b' }, + { op: 'move', from: '/c', path: '/c' } + ]) + + assert.deepStrictEqual(sortObjectKeys(object, undefined, 1), [ + { op: 'move', from: '/a', path: '/a' }, + { op: 'move', from: '/b', path: '/b' }, + { op: 'move', from: '/c', path: '/c' } + ]) + + assert.deepStrictEqual(sortObjectKeys(object, undefined, -1), [ + { op: 'move', from: '/c', path: '/c' }, + { op: 'move', from: '/b', path: '/b' }, + { op: 'move', from: '/a', path: '/a' } + ]) + }) + + it('should sort object keys using a rootPath', () => { + const object = { b: 1, c: 1, a: 1 } + + assert.deepStrictEqual(sortObjectKeys(object, ['root', 'path']), [ + { op: 'move', from: '/root/path/a', path: '/root/path/a' }, + { op: 'move', from: '/root/path/b', path: '/root/path/b' }, + { op: 'move', from: '/root/path/c', path: '/root/path/c' } + ]) + }) + + it('should sort object keys case insensitive', () => { + const object = { B: 1, a: 1 } + + assert.deepStrictEqual(sortObjectKeys(object), [ + { op: 'move', from: '/a', path: '/a' }, + { op: 'move', from: '/B', path: '/B' } + ]) + }) + + it('should sort array', () => { + assert.deepStrictEqual(sortArray([2, 3, 1]), [ + { op: 'move', from: '/2', path: '/0' } + ]) + assert.deepStrictEqual(sortArray([2, 3, 1], undefined, undefined, -1), [ + { op: 'move', from: '/1', path: '/0' } + ]) + }) + + it('should sort array using natural sort', () => { + assert.deepStrictEqual(sortArray(['10', '2', '1']), [ + { op: 'move', from: '/1', path: '/0' }, + { op: 'move', from: '/2', path: '/0' } + ]) + }) + + it('should sort array case insensitive', () => { + assert.deepStrictEqual(sortArray(['B', 'a']), [ + { op: 'move', from: '/1', path: '/0' } + ]) + }) + + it('should sort array using a rootPath', () => { + assert.deepStrictEqual(sortArray([2, 3, 1], ['root', 'path']), [ + { op: 'move', from: '/root/path/2', path: '/root/path/0' } + ]) + }) + + it('should sort array by nested properties and custom direction', () => { + const a = { data: { value: 1 } } + const b = { data: { value: 2 } } + const c = { data: { value: 3 } } + + assert.deepStrictEqual(sortArray([b, a, c], undefined, ['data', 'value']), [ + { op: 'move', from: '/1', path: '/0' } + ]) + assert.deepStrictEqual(sortArray([b, a, c], undefined, ['data', 'value'], 1), [ + { op: 'move', from: '/1', path: '/0' } + ]) + assert.deepStrictEqual(sortArray([b, a, c], undefined, ['data', 'value'], -1), [ + { op: 'move', from: '/2', path: '/0' } + ]) }) it('should give the move operations needed to sort given array', () => { const comparator = (a, b) => a - b - assert.deepStrictEqual(sortMoveOperations([ 1, 2, 3 ], comparator), []) + assert.deepStrictEqual(getSortingMoves([1, 2, 3], comparator), []) - assert.deepStrictEqual(sortMoveOperations([ 2, 3, 1 ], comparator), [ + assert.deepStrictEqual(getSortingMoves([2, 3, 1], comparator), [ { fromIndex: 2, toIndex: 0 } ]) - assert.deepStrictEqual(sortMoveOperations([ 2, 1, 3 ], comparator), [ + assert.deepStrictEqual(getSortingMoves([2, 1, 3], comparator), [ { fromIndex: 1, toIndex: 0 } ]) - assert.deepStrictEqual(sortMoveOperations([ 1, 3, 2 ], comparator), [ + assert.deepStrictEqual(getSortingMoves([1, 3, 2], comparator), [ { fromIndex: 2, toIndex: 1 } ]) - assert.deepStrictEqual(sortMoveOperations([ 3, 2, 1 ], comparator), [ + assert.deepStrictEqual(getSortingMoves([3, 2, 1], comparator), [ { fromIndex: 1, toIndex: 0 }, { fromIndex: 2, toIndex: 0 } ]) - assert.deepStrictEqual(sortMoveOperations([ 3, 1, 2 ], comparator), [ + assert.deepStrictEqual(getSortingMoves([3, 1, 2], comparator), [ { fromIndex: 1, toIndex: 0 }, { fromIndex: 2, toIndex: 1 } ]) }) + + it('should give the move operations needed to sort given array containing objects', () => { + const comparator = (a, b) => a.id - b.id + + const actual = getSortingMoves([{ id: 4 }, { id: 3 }, { id: 1 }, { id: 2 }], comparator) + + const expected = [ + { fromIndex: 1, toIndex: 0 }, + { fromIndex: 2, toIndex: 0 }, + { fromIndex: 3, toIndex: 1 } + ] + + assert.deepStrictEqual(actual, expected) + }) })