-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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/data
#47848
Conversation
Size Change: +5 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
Flaky tests detected in 416c0dc. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4118075198
|
@@ -608,7 +624,7 @@ function mapResolvers( resolvers, selectors, store, resolversCache ) { | |||
* @param {Array} args Selector Arguments. | |||
*/ | |||
async function fulfillResolver( store, resolvers, selectorName, ...args ) { | |||
const resolver = get( resolvers, [ selectorName ] ); | |||
const resolver = resolvers[ selectorName ]; |
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.
I hope that resolvers
can only be an object based on the existing documentation. The only difference with get
is that would gracefully handle different param types here.
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.
It will always be an object. If you trace it to its only usage, you'll notice that it's called on the result of a mapValues()
, and that will generally only be 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.
Looks good. I left one note about get
where I had some minor concerns only because we don't use TypeScript there.
What?
This PR removes the last Lodash from
@wordpress/data
and removes the dependency from the package. There is just a single use in that component and conversion is pretty straightforward.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 #39495We're aiming to reduce the bundle size of
@wordpress/data
since Lodash is over a quarter of its size. This is the last PR of that series.How?
We're using
Object.fromEntries( Object.entries().map() )
as a replacement of_.mapValues()
and direct access instead of_.get()
.Testing Instructions