Lodash: Remove completely from @wordpress/media-utils package
#41967
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
[Feature] Media
tyxla
requested review from
aristath,
gziolo,
swissspidy,
chad1008,
andrewserong,
ntsekouras and
fabiankaegy
|
Size Change: +21 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
tyxla
force-pushed
the
remove/lodash-media-utils
branch
from
5ba6537 to
80a2cbb
Compare
|
Rebased to keep up to date. Would still love to get a review of this one if anyone has spare review cycles 🙌 |
andrewserong
approved these changes
Jul 6, 2022
There was a problem hiding this comment.
Thanks for yet another good removal PR @tyxla! Each of these changes looks good to me visually, and I didn't run into any errors while smoke testing via the Gallery block, and a variety of different Upload / Replace image actions.
LGTM!
|
Thanks for taking a look @andrewserong 🙌 |
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 removes all of the Lodash from the
@wordpress/media-utilspackage, including thelodashdependency altogether. There are just a few usages 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-fetchpackage haslodashas a dependency #39495How?
This PR could serve as a demonstration of why Lodash can be just completely unnecessary and can most often be replaced with elegant JavaScript code. We're dealing with replacing a few methods, namely:
defaultsFor the simple use case, it can be replaced just by creating a new object and using object spread syntax. This is the one and only usage of this method, so we're also deprecating it in ESLint as well.
castArrayReplacing
castArrayis as simple as checking if X is an array, and creating an array with the X element if it's not.pickReplacing
pickis generally pretty easy when we destructure a few object properties we can name, and create an object with them. However, the usage here has a bit more properties, so we're using areduce()call to iterate through them all.compactOne of the most straightforward conversions when it's called with arrays - we can just replace it with
Array.prototype.filter( Boolean ).flatMapWe're replacing it with a
map()on the result ofObject.entries(), followed by a.flat()to flatten the result array.forEachSimple enough to replace with
Array.prototype.forEach()getStraightforward replace with direct access, when necessary we add optional chaining and use nullish coalescing for handling default values.
hasUsually replaced with
Object.hasOwn()orObject.hasOwnProperty(), but in this case it's not necessary, because anErrorobject always has amessage, and even if it doesn't, we're still allowed to access an undefined property.includesSimple enough to replace with
Array.prototype.includes()andString.prototype.includes().mapSimple enough to replace with
Array.prototype.map().omitWe're using destructuring with rest props, which nicely handles the omission.
someSimple enough to replace with
Array.prototype.some()startsWithSimple enough to replace with
String.prototype.startsWith().Testing Instructions
npm run test-unit packages/media-utils/src/utils