Lodash: Refactor some _.isEmpty() instances
#47353
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tyxla
added
[Type] Performance
tyxla
requested review from
Mamaduka,
sgomes,
aristath,
jsnajdr,
swissspidy,
flootr,
chad1008,
ntsekouras,
fabiankaegy and
brookewp
tyxla
commented
Jan 23, 2023
| @@ -25,7 +20,7 @@ export default function FontFamilyControl( { | |||
| fontFamilies = blockLevelFontFamilies; | |||
| } | |||
|
|
|||
| if ( isEmpty( fontFamilies ) ) { | |||
| if ( ! fontFamilies || fontFamilies.length === 0 ) { | |||
There was a problem hiding this comment.
fontFamilies is being used as an array a few lines below already, so making sure it's not nullish and then checking for the array length is a safe bet.
| @@ -39,7 +34,7 @@ export default function ImageSizeControl( { | |||
|
|
|||
| return ( | |||
| <> | |||
| { ! isEmpty( imageSizeOptions ) && ( | |||
| { imageSizeOptions && imageSizeOptions.length > 0 && ( | |||
There was a problem hiding this comment.
SelectControl's options is always an array. However, we're just making sure it's not nullish before actually checking the array length.
| @@ -135,7 +130,7 @@ function InserterSearchResults( { | |||
| ); | |||
|
|
|||
| const hasItems = | |||
| ! isEmpty( filteredBlockTypes ) || ! isEmpty( filteredBlockPatterns ); | |||
| filteredBlockTypes.length > 0 || filteredBlockPatterns.length > 0; | |||
There was a problem hiding this comment.
Already used as arrays in this component, so checking the length is enough.
| @@ -211,5 +206,5 @@ export function isValuesDefined( values ) { | |||
| if ( values === undefined || values === null ) { | |||
| return false; | |||
| } | |||
| return ! isEmpty( Object.values( values ).filter( ( value ) => !! value ) ); | |||
| return Object.values( values ).filter( ( value ) => !! value ).length > 0; | |||
There was a problem hiding this comment.
Object.values() will always return an array, so checking .length here will do the job.
| @@ -41,7 +36,7 @@ export const settings = { | |||
| } | |||
|
|
|||
| if ( context === 'accessibility' ) { | |||
| return isEmpty( content ) | |||
| return ! content || content.length === 0 | |||
There was a problem hiding this comment.
content will be a string, but we're still making sure it's not nullish for some weird reason.
| @@ -35,7 +30,7 @@ export const settings = { | |||
| __experimentalLabel( attributes, { context } ) { | |||
| if ( context === 'accessibility' ) { | |||
| const { content } = attributes; | |||
| return isEmpty( content ) ? __( 'Empty' ) : content; | |||
| return ! content || content.length === 0 ? __( 'Empty' ) : content; | |||
There was a problem hiding this comment.
Same as above for heading's content attribute.
|
Size Change: -2 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
sgomes
approved these changes
Jan 24, 2023
|
There seems to lots more instance of |
|
Thanks for chiming in @spacedmonkey - we're intentionally doing the refactoring in small pieces, in order to be able to:
Also, this PR was focused on addressing simpler use cases of Stay tuned for more of the Lodash removal PRs 💪 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR refactors some of the simpler existing Lodash
_.isEmpty()usages.Why?
Lodash is known to unnecessarily inflate the bundle size of packages, and in most cases, it can be replaced with native language functionality. See these for more information and rationale:
@wordpress/api-fetchpackage haslodashas a dependency #39495How?
We're using simpler native terms of checking whether the item is empty in the current context. I'm going to post a self-review that adds more information about each change.
Testing Instructions