-
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: Refactor away from _.set()
in getNodesWithSettings()
#52278
Conversation
Size Change: -5 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
PRESET_METADATA.forEach( ( { path } ) => { | ||
const value = get( treeToPickFrom, path, false ); | ||
if ( value !== false ) { | ||
set( presets, path, value ); | ||
presets = setImmutably( presets, path, value ); |
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.
Pretty straightforward to trace - presets
is already declared to be an object, and path
is already an array of strings (see PRESET_METADATA
declaration).
We needed to add support for arrays in order to properly work with root and nested arrays.
Flaky tests detected in d491d6e56e45ca86800be67cd00504a0d43c8234. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5454375786
|
d491d6e
to
6f36e49
Compare
6f36e49
to
0e44ec1
Compare
Ready for a review after #52280 landed. |
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. I couldn't spot any regressions ✅
What?
This PR removes Lodash's
_.set()
from thegetNodesWithSettings()
global styles function.We have a few more usages of
set()
left in the codebase, but we might have to tread carefully, since some of them may have to deal with Unicode characters, and I prefer testing them one by one.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 replacing the usage with the existing
setImmutably
utility function, which should do the job even better - there's no need to specifically mutate, as long as we preserve all the prior changes.Testing Instructions