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.

Sign up for GitHub

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 _.uniq() #43330

Merged
merged 3 commits into from
Aug 18, 2022
Merged

Lodash: Refactor away from _.uniq() #43330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23dfeb7
Lodash: Refactor away from _.uniq()
tyxla Aug 17, 2022
de9aa2a
Add changelog
tyxla Aug 17, 2022
ad02c59
Use Set.size
tyxla Aug 17, 2022
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ module.exports = {
'toString',
'trim',
'truncate',
'uniq',
'uniqBy',
'uniqueId',
'uniqWith',
Expand Down
18 changes: 10 additions & 8 deletions bin/plugin/commands/changelog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
const { groupBy, escapeRegExp, uniq, flow, sortBy } = require( 'lodash' );
const { groupBy, escapeRegExp, flow, sortBy } = require( 'lodash' );
const Octokit = require( '@octokit/rest' );
const { sprintf } = require( 'sprintf-js' );
const semver = require( 'semver' );
Expand Down Expand Up @@ -194,13 +194,15 @@ const REWORD_TERMS = {
* @return {string[]} Type candidates.
*/
function getTypesByLabels( labels ) {
return uniq(
labels
.filter( ( label ) =>
Object.keys( LABEL_TYPE_MAPPING ).includes( label )
)
.map( ( label ) => LABEL_TYPE_MAPPING[ label ] )
);
return [
...new Set(
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, is there any difference between spreading the Set and using Array.from?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question! I guess the answer has a few different angles:

  • Performance - they're almost equal, spreading being a bit faster: https://jsben.ch/5lKjg
  • Functionality - they work the same way when working with iterables (which Sets are).
  • Transpilation - the end result will differ slightly, but that would matter if this code was transpiled (which it isn't).

Unless you have strong feelings, I'd prefer to use spread as it's way more common around the codebase.

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2022-08-17 at 17 51 24

Safari, macOS, so I guess it depends but it's nearly the same.

I don't have any objections to using spread here. In general, both approaches discussed are less readable than uniq. It isn't immediately obvious that Set is used to remove duplicates.

Copy link
Contributor

Choose a reason for hiding this comment

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

It isn't immediately obvious that Set is used to remove duplicates.

I don't know if I agree with that; [ ...new Set() ] is a pretty common idiom these days.

labels
.filter( ( label ) =>
Object.keys( LABEL_TYPE_MAPPING ).includes( label )
)
.map( ( label ) => LABEL_TYPE_MAPPING[ label ] )
),
];
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/babel-plugin-makepot/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const { po } = require( 'gettext-parser' );
const {
pick,
reduce,
uniq,
forEach,
sortBy,
isEqual,
Expand Down Expand Up @@ -347,8 +346,8 @@ module.exports = () => {
memo[ msgctxt ][ msgid ]
)
) {
translation.comments.reference =
uniq(
translation.comments.reference = [
...new Set(
[
memo[ msgctxt ][ msgid ]
.comments.reference,
Expand All @@ -357,7 +356,8 @@ module.exports = () => {
]
.join( '\n' )
.split( '\n' )
).join( '\n' );
),
].join( '\n' );
}

memo[ msgctxt ][ msgid ] = translation;
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/block-switcher/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { castArray, uniq } from 'lodash';
import { castArray } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -64,7 +64,7 @@ export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
_icon = blockInformation?.icon; // Take into account active block variations.
} else {
const isSelectionOfSameType =
uniq( blocks.map( ( { name } ) => name ) ).length === 1;
new Set( blocks.map( ( { name } ) => name ) ).size === 1;
// When selection consists of blocks of multiple types, display an
// appropriate icon to communicate the non-uniformity.
_icon = isSelectionOfSameType
Expand Down
25 changes: 11 additions & 14 deletions packages/block-editor/src/components/date-format-picker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { uniq } from 'lodash';

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -83,15 +78,17 @@ function NonDefaultControls( { format, onChange } ) {
// 2022) in German (de). The resultant array is de-duplicated as some
// languages will use the same format string for short, medium, and long
// formats.
const suggestedFormats = uniq( [
'Y-m-d',
_x( 'n/j/Y', 'short date format' ),
_x( 'n/j/Y g:i A', 'short date format with time' ),
_x( 'M j, Y', 'medium date format' ),
_x( 'M j, Y g:i A', 'medium date format with time' ),
_x( 'F j, Y', 'long date format' ),
_x( 'M j', 'short date format without the year' ),
] );
const suggestedFormats = [
...new Set( [
'Y-m-d',
_x( 'n/j/Y', 'short date format' ),
_x( 'n/j/Y g:i A', 'short date format with time' ),
_x( 'M j, Y', 'medium date format' ),
_x( 'M j, Y g:i A', 'medium date format with time' ),
_x( 'F j, Y', 'long date format' ),
_x( 'M j', 'short date format without the year' ),
] ),
];

const suggestedOptions = suggestedFormats.map(
( suggestedFormat, index ) => ( {
Expand Down
15 changes: 6 additions & 9 deletions packages/block-editor/src/hooks/generated-class-name.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { uniq } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -26,10 +21,12 @@ export function addGeneratedClassName( extraProps, blockType ) {
// We have some extra classes and want to add the default classname
// We use uniq to prevent duplicate classnames.

extraProps.className = uniq( [
getBlockDefaultClassName( blockType.name ),
...extraProps.className.split( ' ' ),
] )
extraProps.className = [
...new Set( [
getBlockDefaultClassName( blockType.name ),
...extraProps.className.split( ' ' ),
] ),
]
.join( ' ' )
.trim();
} else {
Expand Down
11 changes: 6 additions & 5 deletions packages/blocks/src/api/factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
filter,
first,
has,
uniq,
isEmpty,
map,
} from 'lodash';
Expand Down Expand Up @@ -348,10 +347,12 @@ export function getPossibleBlockTransformations( blocks ) {
const blockTypesForToTransforms =
getBlockTypesForPossibleToTransforms( blocks );

return uniq( [
...blockTypesForFromTransforms,
...blockTypesForToTransforms,
] );
return [
...new Set( [
...blockTypesForFromTransforms,
...blockTypesForToTransforms,
] ),
];
}

/**
Expand Down
3 changes: 3 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
- `DateTimePicker`, `TimePicker`, `DatePicker`: Switch from `moment` to `date-fns` ([#43005](https://github.com/WordPress/gutenberg/pull/43005)).
- `DatePicker`: Switch from `react-dates` to `use-lilius` ([#43005](https://github.com/WordPress/gutenberg/pull/43005)).
- `convertLTRToRTL()`: Refactor away from `_.mapKeys()` ([#43258](https://github.com/WordPress/gutenberg/pull/43258/)).
- `FormTokenField`: Refactor away from `_.uniq()` ([#43330](https://github.com/WordPress/gutenberg/pull/43330/)).
- `contextConnect`: Refactor away from `_.uniq()` ([#43330](https://github.com/WordPress/gutenberg/pull/43330/)).
- `ColorPalette`: Refactor away from `_.uniq()` ([#43330](https://github.com/WordPress/gutenberg/pull/43330/)).

## 19.17.0 (2022-08-10)

Expand Down
16 changes: 10 additions & 6 deletions packages/components/src/color-palette/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
Platform,
Text,
} from 'react-native';
import { map, uniq } from 'lodash';
import { map } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -63,11 +63,15 @@ function ColorPalette( {
const scale = useRef( new Animated.Value( 1 ) ).current;
const opacity = useRef( new Animated.Value( 1 ) ).current;

const defaultColors = uniq( map( defaultSettings.colors, 'color' ) );
const mergedColors = uniq( map( defaultSettings.allColors, 'color' ) );
const defaultGradientColors = uniq(
map( defaultSettings.gradients, 'gradient' )
);
const defaultColors = [
...new Set( map( defaultSettings.colors, 'color' ) ),
];
const mergedColors = [
...new Set( map( defaultSettings.allColors, 'color' ) ),
];
const defaultGradientColors = [
...new Set( map( defaultSettings.gradients, 'gradient' ) ),
];
const colors = isGradientSegment ? defaultGradientColors : defaultColors;

const customIndicatorColor = isGradientSegment
Expand Down
16 changes: 9 additions & 7 deletions packages/components/src/form-token-field/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { last, clone, uniq, map, some } from 'lodash';
import { last, clone, map, some } from 'lodash';
import classnames from 'classnames';
import type { KeyboardEvent, MouseEvent, TouchEvent } from 'react';

Expand Down Expand Up @@ -409,12 +409,14 @@ export function FormTokenField( props: FormTokenFieldProps ) {
}

function addNewTokens( tokens: string[] ) {
const tokensToAdd = uniq(
tokens
.map( saveTransform )
.filter( Boolean )
.filter( ( token ) => ! valueContainsToken( token ) )
);
const tokensToAdd = [
...new Set(
tokens
.map( saveTransform )
.filter( Boolean )
.filter( ( token ) => ! valueContainsToken( token ) )
),
];

if ( tokensToAdd.length > 0 ) {
const newValue = clone( value );
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/ui/context/context-connect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* External dependencies
*/
import { uniq } from 'lodash';
import type { ForwardedRef, ReactChild, ReactNode } from 'react';

/**
Expand Down Expand Up @@ -70,7 +69,9 @@ export function contextConnect< P >(
WrappedComponent.displayName = namespace;

// @ts-ignore internal property
WrappedComponent[ CONNECT_STATIC_NAMESPACE ] = uniq( mergedNamespace );
WrappedComponent[ CONNECT_STATIC_NAMESPACE ] = [
...new Set( mergedNamespace ),
];

// @ts-ignore WordPressComponent property
WrappedComponent.selector = `.${ getStyledClassNameFromKey( namespace ) }`;
Expand Down
15 changes: 8 additions & 7 deletions test/integration/fixtures/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import fs from 'fs';
import path from 'path';
import { uniq } from 'lodash';

const FIXTURES_DIR = path.join( __dirname, 'blocks' );

Expand Down Expand Up @@ -31,12 +30,14 @@ export function getAvailableBlockFixturesBasenames() {
// - fixture.json : blocks structure
// - fixture.serialized.html : re-serialized content
// Get the "base" name for each fixture first.
return uniq(
fs
.readdirSync( FIXTURES_DIR )
.filter( ( f ) => /(\.html|\.json)$/.test( f ) )
.map( ( f ) => f.replace( /\..+$/, '' ) )
);
return [
...new Set(
fs
.readdirSync( FIXTURES_DIR )
.filter( ( f ) => /(\.html|\.json)$/.test( f ) )
.map( ( f ) => f.replace( /\..+$/, '' ) )
),
];
}

/**
Expand Down