-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Remove from @wordpress/editor
package
#49799
Conversation
@@ -38,7 +33,7 @@ export function PostTemplate() { | |||
|
|||
const { editPost } = useDispatch( editorStore ); | |||
|
|||
if ( ! isViewable || isEmpty( availableTemplates ) ) { | |||
if ( ! isViewable || ! Object.entries( availableTemplates ?? {} ).length ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
availableTemplates
could potentially be nullish, so we're falling back to an empty object before checking if it's empty. If not nullish, it is always an object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using Object.keys
? It creates a smaller array, so intuitively it should be less wasteful 🙂
Instead of the ?? {}
suffix I'd consider writing isViewable || ! availableTemplates || ! Object.keys( ... ).length
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both are good suggestions, thanks 👍 Addressed in 2f1bf4d
@@ -87,7 +82,7 @@ const createWithMetaAttributeSource = ( metaAttributes ) => | |||
] ) | |||
); | |||
|
|||
if ( ! isEmpty( nextMeta ) ) { | |||
if ( Object.entries( nextMeta ).length ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nextMeta
is always an object as defined a few lines above (L71).
@@ -115,7 +110,7 @@ function shimAttributeSource( settings ) { | |||
.filter( ( [ , { source } ] ) => source === 'meta' ) | |||
.map( ( [ attributeKey, { meta } ] ) => [ attributeKey, meta ] ) | |||
); | |||
if ( ! isEmpty( metaAttributes ) ) { | |||
if ( Object.entries( metaAttributes ).length ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metaAttributes
is always an object, as defined a few lines above (L108).
Size Change: -6 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -38,7 +33,7 @@ export function PostTemplate() { | |||
|
|||
const { editPost } = useDispatch( editorStore ); | |||
|
|||
if ( ! isViewable || isEmpty( availableTemplates ) ) { | |||
if ( ! isViewable || ! Object.entries( availableTemplates ?? {} ).length ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using Object.keys
? It creates a smaller array, so intuitively it should be less wasteful 🙂
Instead of the ?? {}
suffix I'd consider writing isViewable || ! availableTemplates || ! Object.keys( ... ).length
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀🚢
b538784
to
18ae886
Compare
What?
This PR removes the remaining Lodash from the
@wordpress/editor
package and removes the unused dependency. There were just a few straightforwardisEmpty()
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-fetch
package haslodash
as a dependency #39495How?
We're using
Object.entries( object ).length
as an alternative, with a nullish coalescing fallback when necessary.Testing Instructions
_.mapValues()
from editor package #49654 and verify they still work.