Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lodash: Refactor away from _.orderBy() #45146

Merged
merged 7 commits into from
Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ const restrictedImports = [
'nth',
'omitBy',
'once',
'orderby',
'overEvery',
'partial',
'partialRight',
Expand Down
8 changes: 2 additions & 6 deletions packages/block-editor/src/autocompleters/block.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { orderBy } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -20,6 +15,7 @@ import { searchBlockItems } from '../components/inserter/search-items';
import useBlockTypesState from '../components/inserter/hooks/use-block-types-state';
import BlockIcon from '../components/block-icon';
import { store as blockEditorStore } from '../store';
import { orderBy } from '../utils/sorting';

const noop = () => {};
const SHOWN_BLOCK_TYPES = 9;
Expand Down Expand Up @@ -68,7 +64,7 @@ function createBlockCompleter() {
collections,
filterValue
)
: orderBy( items, [ 'frecency' ], [ 'desc' ] );
: orderBy( items, 'frecency', 'desc' );

return initialFilteredItems
.filter( ( item ) => item.name !== selectedBlockName )
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/**
* External dependencies
* WordPress dependencies
*/
import { orderBy } from 'lodash';
import { createContext, useContext } from '@wordpress/element';

/**
* WordPress dependencies
* Internal dependencies
*/
import { createContext, useContext } from '@wordpress/element';
import { orderBy } from '../../utils/sorting';

export const DEFAULT_BLOCK_LIST_CONTEXT = {
scrollRef: null,
Expand Down Expand Up @@ -103,10 +103,7 @@ export function deleteBlockLayoutByClientId( data, clientId ) {
*/
function getBlockLayoutsOrderedByYCoord( data ) {
// Only enabled for root level blocks.
// Using lodash orderBy due to hermes not having
// stable support for native .sort(). It will be
// supported in the React Native version 0.68.0.
return orderBy( data, [ 'y', 'asc' ] );
return orderBy( Object.values( data ), 'y' );
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { map, groupBy, orderBy } from 'lodash';
import { map, groupBy } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -17,6 +17,7 @@ import BlockTypesList from '../block-types-list';
import InserterPanel from './panel';
import useBlockTypesState from './hooks/use-block-types-state';
import InserterListbox from '../inserter-listbox';
import { orderBy } from '../../utils/sorting';

const getBlockNamespace = ( item ) => item.name.split( '/' )[ 0 ];

Expand All @@ -42,7 +43,7 @@ export function BlockTypesTab( {
);

const suggestedItems = useMemo( () => {
return orderBy( items, [ 'frecency' ], [ 'desc' ] ).slice(
return orderBy( items, 'frecency', 'desc' ).slice(
0,
MAX_SUGGESTED_ITEMS
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { orderBy, isEmpty } from 'lodash';
import { isEmpty } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -25,6 +25,7 @@ import usePatternsState from './hooks/use-patterns-state';
import useBlockTypesState from './hooks/use-block-types-state';
import { searchBlockItems, searchItems } from './search-items';
import InserterListbox from '../inserter-listbox';
import { orderBy } from '../../utils/sorting';

const INITIAL_INSERTER_RESULTS = 9;
/**
Expand Down Expand Up @@ -91,7 +92,7 @@ function InserterSearchResults( {
return [];
}
const results = searchBlockItems(
orderBy( blockTypes, [ 'frecency' ], [ 'desc' ] ),
orderBy( blockTypes, 'frecency', 'desc' ),
blockTypeCategories,
blockTypeCollections,
filterValue
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
/**
* External dependencies
*/

import { orderBy } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
*/

import { __ } from '@wordpress/i18n';
import { ToolbarItem, DropdownMenu, Slot } from '@wordpress/components';
import { chevronDown } from '@wordpress/icons';

/**
* Internal dependencies
*/
import { orderBy } from '../../../utils/sorting';

const POPOVER_PROPS = {
position: 'bottom right',
variant: 'toolbar',
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { map, find, orderBy } from 'lodash';
import { map, find } from 'lodash';
import createSelector from 'rememo';

/**
Expand All @@ -27,6 +27,7 @@ import deprecated from '@wordpress/deprecated';
* Internal dependencies
*/
import { mapRichTextSettings } from './utils';
import { orderBy } from '../utils/sorting';

/**
* A block selection object.
Expand Down
54 changes: 54 additions & 0 deletions packages/block-editor/src/utils/sorting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* Recursive stable sorting comparator function.
*
* @param {string|Function} field Field to sort by.
* @param {Array} items Items to sort.
* @param {string} order Order, 'asc' or 'desc'.
* @return {Function} Comparison function to be used in a `.sort()`.
*/
const comparator = ( field, items, order ) => {
return ( a, b ) => {
let cmpA, cmpB;

if ( typeof field === 'function' ) {
cmpA = field( a );
cmpB = field( b );
} else {
cmpA = a[ field ];
cmpB = b[ field ];
}

if ( cmpA > cmpB ) {
return order === 'asc' ? 1 : -1;
} else if ( cmpB > cmpA ) {
return order === 'asc' ? -1 : 1;
}

const orderA = items.findIndex( ( item ) => item === a );
const orderB = items.findIndex( ( item ) => item === b );

// Stable sort: maintaining original array order
if ( orderA > orderB ) {
return 1;
} else if ( orderB > orderA ) {
return -1;
}
Comment on lines +30 to +35
Copy link
Member Author

@tyxla tyxla Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how Lodash _.orderBy() works - it maintains original array order, regardless of whether we're sorting in ascending or descending order:

Screenshot 2022-12-02 at 16 17 41

We're opting to keep that behavior and supporting it with a couple of unit tests.


return 0;
};
};

/**
* Order items by a certain key.
* Supports decorator functions that allow complex picking of a comparison field.
* Sorts in ascending order by default, but supports descending as well.
* Stable sort - maintains original order of equal items.
*
* @param {Array} items Items to order.
* @param {string|Function} field Field to order by.
* @param {string} order Sorting order, `asc` or `desc`.
* @return {Array} Sorted items.
*/
export function orderBy( items, field, order = 'asc' ) {
return items.concat().sort( comparator( field, items, order ) );
}
49 changes: 49 additions & 0 deletions packages/block-editor/src/utils/test/sorting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Internal dependencies
*/
import { orderBy } from '../sorting';

describe( 'orderBy', () => {
it( 'should not mutate original input', () => {
const input = [];
expect( orderBy( input, 'x' ) ).not.toBe( input );
} );

it( 'should sort items by a field when it is specified as a string', () => {
const input = [ { x: 2 }, { x: 1 }, { x: 3 } ];
const expected = [ { x: 1 }, { x: 2 }, { x: 3 } ];
expect( orderBy( input, 'x' ) ).toEqual( expected );
} );

it( 'should support functions for picking the field', () => {
const input = [ { x: 2 }, { x: 1 }, { x: 3 } ];
const expected = [ { x: 1 }, { x: 2 }, { x: 3 } ];
expect( orderBy( input, ( item ) => item.x ) ).toEqual( expected );
} );

it( 'should support sorting in a descending order', () => {
const input = [ { x: 2 }, { x: 1 }, { x: 3 } ];
const expected = [ { x: 3 }, { x: 2 }, { x: 1 } ];
expect( orderBy( input, 'x', 'desc' ) ).toEqual( expected );
} );

it( 'should maintain original order of equal items', () => {
const a = { x: 1, a: 1 };
const b = { x: 1, b: 2 };
const c = { x: 0 };
const d = { x: 1, b: 4 };
const input = [ a, b, c, d ];
const expected = [ c, a, b, d ];
expect( orderBy( input, 'x' ) ).toEqual( expected );
} );

it( 'should maintain original order of equal items in descencing order', () => {
const a = { x: 1, a: 1 };
const b = { x: 1, b: 2 };
const c = { x: 0 };
const d = { x: 1, b: 4 };
const input = [ a, b, c, d ];
const expected = [ a, b, d, c ];
expect( orderBy( input, 'x', 'desc' ) ).toEqual( expected );
} );
} );