-
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 completely from site editor #52480
Conversation
@@ -32,7 +35,7 @@ import { blockMeta, post, archive } from '@wordpress/icons'; | |||
export const mapToIHasNameAndId = ( entities, path ) => { | |||
return ( entities || [] ).map( ( entity ) => ( { | |||
...entity, | |||
name: decodeEntities( get( entity, path ) ), | |||
name: decodeEntities( getValueFromObjectPath( entity, path ) ), |
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.
path
is expected to be a string, and for entity
we're handling nullish values just in case.
recordNamePath: 'title.rendered', |
@@ -115,7 +118,7 @@ function useChangesToPush( name, attributes ) { | |||
]; | |||
const value = presetAttributeValue | |||
? `var:preset|${ STYLE_PATH_TO_CSS_VAR_INFIX[ presetAttributeKey ] }|${ presetAttributeValue }` | |||
: get( attributes.style, path ); | |||
: getValueFromObjectPath( attributes.style, path ); |
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.
path
is expected to be an array, and for attributes.style
we're handling nullish values just in case:
const presetAttributeKey = path.join( '.' ); |
@@ -71,7 +71,6 @@ | |||
"downloadjs": "^1.4.7", | |||
"fast-deep-equal": "^3.1.3", | |||
"is-plain-object": "^5.0.0", | |||
"lodash": "^4.17.21", |
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.
One of the last packages to remove the dependency from 🥳
Size Change: +49 B (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
Flaky tests detected in 588b5cb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5510853269
|
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.
What?
This PR removes Lodash's
_.get()
from the@wordpress/edit-site
package and since those are the last Lodash usages there, it removes the Lodash dependency.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 direct access with optional chaining as an alternative. It might feel like we're being repetitive adding the same function twice, but it's not the same actually: in one of the cases it works with an array path, and in the other case, it works with a string path. There are other usages and different implementations of
getValueFromObjectPath
throughout the codebase and I'm planning to follow up with unifying them in another PR.Testing Instructions
_.set()
inPushChangesToGlobalStylesControl
#52404.