fix(compat/orderby): add missed test cases and fix the problematic parts (#427)

* Delete getPath

* Add testcase and fix orderBy

* Fix jsdoc

* Fix jsdoc

* Fix comment

* Change variable name

* Improve performance

* Remove unused

* Remove memoize by change logic

* Remove unusable type

* Add testcase for coverage

* Refactor code

* Extract condition

* Use object only needed

* Change variable name and integrate map

* Apply suggestions from code review

---------

Co-authored-by: Sojin Park <raon0211@gmail.com>
This commit is contained in:
Dayong Lee 2024-08-31 14:31:48 +09:00 committed by GitHub
parent 5839385476
commit 27b483e945
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 214 additions and 122 deletions

View File

@ -9,41 +9,39 @@ const users = [
{ user: 'fred', age: 40, nested: { user: 'fred' } },
{ user: 'barney', age: 36, nested: { user: 'bar' } },
];
const keys: Array<keyof (typeof users)[0]> = ['user', 'age'];
const orders: Array<'asc' | 'desc'> = ['asc', 'desc'];
describe('orderBy', () => {
bench('es-toolkit/orderBy', () => {
orderByToolkit(users, keys, orders);
orderByToolkit(users, [user => user.user, user => user.age], orders);
orderByToolkit(users, ['user', 'age'], ['asc', 'desc']);
orderByToolkit(users, [user => user.user, user => user.age], ['asc', 'desc']);
});
bench('es-toolkit/compat/orderBy', () => {
orderByToolkitCompat(users, keys, orders);
orderByToolkitCompat(users, [user => user.user, user => user.age], orders);
orderByToolkitCompat(users, ['user', 'age'], ['asc', 'desc']);
orderByToolkitCompat(users, [user => user.user, user => user.age], ['asc', 'desc']);
});
bench('lodash/orderBy', () => {
orderByLodash(users, keys, orders);
orderByLodash(users, [user => user.user, user => user.age], orders);
orderByLodash(users, ['user', 'age'], ['asc', 'desc']);
orderByLodash(users, [user => user.user, user => user.age], ['asc', 'desc']);
});
});
describe('orderBy (nested property names)', () => {
bench('es-toolkit/compat/orderBy', () => {
orderByToolkitCompat(users, [['nested', 'user'], ['age']], orders);
orderByToolkitCompat(users, [['nested', 'user'], ['age']], ['asc', 'desc']);
});
bench('lodash/orderBy', () => {
orderByLodash(users, [['nested', 'user'], ['age']], orders);
orderByLodash(users, [['nested', 'user'], ['age']], ['asc', 'desc']);
});
});
describe('orderBy (property path)', () => {
bench('es-toolkit/compat/orderBy', () => {
orderByToolkitCompat(users, ['nested.user', 'age'], orders);
orderByToolkitCompat(users, ['nested.user', 'age'], ['asc', 'desc']);
});
bench('lodash/orderBy', () => {
orderByLodash(users, ['nested.user', 'age'], orders);
orderByLodash(users, ['nested.user', 'age'], ['asc', 'desc']);
});
});

View File

@ -0,0 +1,25 @@
import { describe, it, expect } from 'vitest';
import { compareValues } from './compareValues';
describe('compareValues', () => {
it('should return -1', () => {
expect(compareValues(1, 2, 'asc')).toBe(-1);
expect(compareValues(2, 1, 'desc')).toBe(-1);
expect(compareValues(null, 1, 'asc')).toBe(-1);
expect(compareValues(1, null, 'asc')).toBe(-1);
});
it('should return 1', () => {
expect(compareValues(2, 1, 'asc')).toBe(1);
expect(compareValues(1, 2, 'desc')).toBe(1);
expect(compareValues(1, null, 'desc')).toBe(1);
expect(compareValues(null, 1, 'desc')).toBe(1);
});
it('should return 0', () => {
expect(compareValues(1, 1, 'asc')).toBe(0);
expect(compareValues(1, 1, 'desc')).toBe(0);
expect(compareValues(null, null, 'asc')).toBe(0);
expect(compareValues(null, null, 'desc')).toBe(0);
});
});

View File

@ -0,0 +1,11 @@
export const compareValues = <V>(a: V, b: V, order: string) => {
if ((a != null && b == null) || a < b) {
return order === 'desc' ? 1 : -1;
}
if ((a == null && b != null) || a > b) {
return order === 'desc' ? -1 : 1;
}
return 0;
};

View File

@ -1,24 +0,0 @@
import { describe, it, expect } from 'vitest';
import { getPath } from './getPath';
describe('getPath function', () => {
it('should return a property name from a string', () => {
expect(getPath('a', { a: 1 })).toBe('a');
});
it('should return an array of property names from a deep path string', () => {
expect(getPath('a.b.c', { a: { b: { c: 1 } } })).toEqual(['a', 'b', 'c']);
});
it('should return an array of property names from a string array', () => {
expect(getPath(['a', 'b', 'c'], { a: { b: { c: 1 } } })).toEqual(['a', 'b', 'c']);
});
it('should return an array of property names from a string array with a nested string', () => {
expect(getPath(['a', 'b.c', 'd'], { a: { b: { c: { d: 1 } } } })).toEqual(['a', 'b', 'c', 'd']);
});
it('should return an array of property names from a deep path like string array', () => {
expect(getPath(['a.b.c', 'd'], { 'a.b.c': { d: 1 } })).toEqual(['a.b.c', 'd']);
});
});

View File

@ -1,44 +0,0 @@
import { isKey } from './isKey';
import { toPath } from './toPath';
/**
* Get the `path` (property name) from the `key` (property name or property path).
*
* @template T - The type of the object.
* @param {((item: T) => unknown) | string | string[]} key - The `key` (property name or property path or custom key function) to query.
* @param {T} object - The object to query.
* @returns {((item: T) => unknown) | string | string[]} The converted key (only property name or custom key function).
*/
export function getPath<T extends object>(
key: ((item: T) => unknown) | string | string[],
object: T
): ((item: T) => unknown) | string | string[] {
if (Array.isArray(key)) {
const path = [];
let target: object = object;
for (let i = 0; i < key.length; i++) {
const k = key[i];
if (isKey(k, target)) {
target = target[k as keyof typeof target];
path.push(k);
} else {
const keys = toPath(k);
for (let i = 0; i < keys.length; i++) {
target = target[keys[i] as keyof typeof target];
path.push(keys[i]);
}
}
}
return path;
}
if (typeof key === 'function' || isKey(key, object)) {
return key;
}
return toPath(key);
}

View File

@ -1,8 +1,21 @@
import { describe, expect, it } from 'vitest';
import { orderBy } from './orderBy.ts';
import { falsey } from '../_internal/falsey.ts';
import { zipObject } from '../../array/zipObject.ts';
import { partialRight } from '../../function/partialRight.ts';
describe('orderBy', () => {
class Pair {
constructor(
public a: number | undefined,
public b: number,
public c: number
) {
this.a = a;
this.b = b;
this.c = c;
}
}
const objects = [
{ a: 'x', b: 3 },
{ a: 'y', b: 4 },
@ -10,6 +23,31 @@ describe('orderBy', () => {
{ a: 'y', b: 2 },
];
const stableArray = [
new Pair(1, 1, 1),
new Pair(1, 2, 1),
new Pair(1, 1, 1),
new Pair(1, 2, 1),
new Pair(1, 3, 1),
new Pair(1, 4, 1),
new Pair(1, 5, 1),
new Pair(1, 6, 1),
new Pair(2, 1, 2),
new Pair(2, 2, 2),
new Pair(2, 3, 2),
new Pair(2, 4, 2),
new Pair(2, 5, 2),
new Pair(2, 6, 2),
new Pair(undefined, 1, 1),
new Pair(undefined, 2, 1),
new Pair(undefined, 3, 1),
new Pair(undefined, 4, 1),
new Pair(undefined, 5, 1),
new Pair(undefined, 6, 1),
];
const stableObject = zipObject('abcdefghijklmnopqrst'.split(''), stableArray);
const nestedObj = [
{ id: '4', address: { zipCode: 4, streetName: 'Beta' } },
{ id: '3', address: { zipCode: 3, streetName: 'Alpha' } },
@ -18,6 +56,56 @@ describe('orderBy', () => {
{ id: '5', address: { zipCode: 4, streetName: 'Alpha' } },
];
it(`should sort multiple properties in ascending order`, () => {
const actual = orderBy(objects, ['a', 'b']);
expect(actual).toEqual([objects[2], objects[0], objects[3], objects[1]]);
});
it(`should support iteratees`, () => {
const actual = orderBy(objects, [
'a',
function (object) {
return object.b;
},
]);
expect(actual).toEqual([objects[2], objects[0], objects[3], objects[1]]);
});
it(`should perform a stable sort (test in IE > 8 and V8)`, () => {
expect(orderBy(stableArray, ['a', 'c'])).toEqual(stableArray);
expect(orderBy(stableObject, ['a', 'c'])).toEqual(stableArray);
});
it(`should not error on nullish elements`, () => {
let actual;
try {
actual = orderBy([...objects, null, undefined], ['a', 'b']);
} catch {
// do nothing
}
expect(actual).toEqual([objects[2], objects[0], objects[3], objects[1], null, undefined]);
});
it(`should work as an iteratee for methods like \`_.reduce\``, () => {
const objects = [
{ a: 'x', 0: 3 },
{ a: 'y', 0: 4 },
{ a: 'x', 0: 1 },
{ a: 'y', 0: 2 },
];
expect(['a'].reduce(orderBy, objects)).toEqual([objects[0], objects[2], objects[1], objects[3]]);
expect([0].reduce(orderBy, objects)).toEqual([objects[2], objects[3], objects[0], objects[1]]);
expect([[0]].reduce(orderBy, objects)).toEqual([objects[2], objects[3], objects[0], objects[1]]);
const wrapped = partialRight(orderBy, 'bogus');
expect(['a'].reduce(wrapped, objects)).toEqual([objects[0], objects[2], objects[1], objects[3]]);
expect([0].reduce(wrapped, objects)).toEqual([objects[2], objects[3], objects[0], objects[1]]);
expect([[0]].reduce(wrapped, objects)).toEqual([objects[2], objects[3], objects[0], objects[1]]);
});
it('should return an empty array when the collection is null or undefined', () => {
const actual = [null, undefined].map(value => orderBy(value));
const expected = [[], []];
@ -81,12 +169,25 @@ describe('orderBy', () => {
expect(actual).toEqual(expected);
});
it('should work with `keys` specified as functions', () => {
expect(orderBy(objects, [item => item.a, item => item.b], ['desc', 'asc'])).toEqual([
objects[3],
objects[1],
objects[2],
objects[0],
]);
it('should work with `deep` property paths', () => {
const actual = orderBy(nestedObj, ['address.zipCode'], ['asc']);
const expected = [nestedObj[2], nestedObj[3], nestedObj[1], nestedObj[0], nestedObj[4]];
expect(actual).toEqual(expected);
});
it('should work with nested `deep` property paths when paths length is 1', () => {
const actual = orderBy(nestedObj, [['address.zipCode']], ['asc']);
const expected = [nestedObj[2], nestedObj[3], nestedObj[1], nestedObj[0], nestedObj[4]];
expect(actual).toEqual(expected);
});
it('shoud work with `deep-like` property paths', () => {
const objects = [{ 'a.b': 1 }, { 'a.b': 2 }, { 'a.b': 3 }, { 'a.b': 4 }];
const actual = orderBy(objects, ['a.b'], ['desc']);
const expected = [objects[3], objects[2], objects[1], objects[0]];
expect(actual).toEqual(expected);
});
});

View File

@ -1,16 +1,18 @@
import { getPath } from '../_internal/getPath';
import { compareValues } from '../_internal/compareValues';
import { isKey } from '../_internal/isKey';
import { toPath } from '../_internal/toPath';
/**
* Sorts an array of objects based on multiple properties and their corresponding order directions.
*
* This function takes an array of objects, an array of keys to sort by, and an array of order directions.
* This function takes an array of objects, an array of criteria to sort by, and an array of order directions.
* It returns the sorted array, ordering by each key according to its corresponding direction
* ('asc' for ascending or 'desc' for descending). If values for a key are equal,string
* it moves to the next key to determine the order.
*
* @template T - The type of elements in the array.
* @param {T[] | null} collection - The array of objects to be sorted.
* @param {((item: T) => unknown) | string | Array<((item: T) => unknown) | string | string[]>} keys - An array of keys (property names or property paths or custom key functions) to sort by.
* @param { T[] | object | null | undefined} collection - The array of objects to be sorted.
* @param {((item: T) => unknown) | PropertyKey | Array<((item: T) => unknown) | PropertyKey | PropertyKey[]>} criteria - An array of criteria (property names or property paths or custom key functions) to sort by.
* @param {unknown | unknown[]} orders - An array of order directions ('asc' for ascending or 'desc' for descending).
* @returns {T[]} - The sorted array.
*
@ -31,62 +33,85 @@ import { getPath } from '../_internal/getPath';
* // { user: 'fred', age: 40 },
* // ]
*/
export function orderBy<T extends object>(
collection: T[] | null | undefined,
keys?: ((item: T) => unknown) | string | Array<((item: T) => unknown) | string | string[]>,
export function orderBy<T>(
collection: T[] | object | null | undefined,
criteria?: ((item: T) => unknown) | PropertyKey | Array<((item: T) => unknown) | PropertyKey | PropertyKey[]>,
orders?: unknown | unknown[]
): T[] {
if (collection == null) {
return [];
}
if (!Array.isArray(keys)) {
keys = keys == null ? [] : [keys];
if (!Array.isArray(collection) && typeof collection === 'object') {
collection = Object.values(collection);
}
if (!Array.isArray(criteria)) {
criteria = criteria == null ? [] : [criteria];
}
if (!Array.isArray(orders)) {
orders = orders == null ? [] : [orders];
}
const compareValues = <V>(a: V, b: V, order: string) => {
if (a < b) {
return order === 'desc' ? 1 : -1; // Default is ascending order
const getValueByNestedPath = (object: object, path: PropertyKey[]) => {
let target: object = object;
for (let i = 0; i < path.length && target != null; i++) {
target = target[path[i] as keyof typeof target];
}
if (a > b) {
return order === 'desc' ? -1 : 1;
}
return 0;
return target;
};
const getValueByPath = (path: string | ((item: T) => unknown) | string[], obj: T) => {
if (Array.isArray(path)) {
let value: object = obj;
for (let i = 0; i < path.length; i++) {
value = value[path[i] as keyof typeof value];
}
return value;
const getValueByCriterion = (
criterion: PropertyKey | ((item: T) => unknown) | PropertyKey[] | { key: PropertyKey; path: string[] },
object: T
) => {
if (object == null) {
return object;
}
if (typeof path === 'function') {
return path(obj);
if (typeof criterion === 'function') {
return criterion(object);
}
return obj[path as keyof typeof obj];
if (Array.isArray(criterion)) {
return getValueByNestedPath(object, criterion);
}
if (typeof criterion !== 'object') {
return object[criterion as keyof typeof object];
}
// Case for possible to be a deep path
if (Object.hasOwn(object, criterion.key)) {
return object[criterion.key as keyof typeof object];
}
return getValueByNestedPath(object, criterion.path);
};
keys = keys.map(key => getPath(key, collection[0]));
// Prepare all cases for criteria
const preparedCriteria = criteria.map(criterion => {
// lodash handles a array with one element as a single criterion
if (Array.isArray(criterion) && criterion.length === 1) {
criterion = criterion[0];
}
return collection.slice().sort((a, b) => {
for (let i = 0; i < keys.length; i++) {
const path = keys[i];
if (typeof criterion === 'function' || Array.isArray(criterion) || isKey(criterion)) {
return criterion;
}
const valueA = getValueByPath(path, a);
const valueB = getValueByPath(path, b);
// If criterion is not key, it has possibility to be a deep path. So we have to prepare both cases.
return { key: criterion, path: toPath(criterion as string) } as const;
});
return (collection as T[]).slice().sort((a, b) => {
for (let i = 0; i < criteria.length; i++) {
const order = String((orders as unknown[])[i]); // For Object('desc') case
const valueA = getValueByCriterion(preparedCriteria[i], a);
const valueB = getValueByCriterion(preparedCriteria[i], b);
const comparedResult = compareValues(valueA, valueB, order);