Lodash: Refactor away from _.omit()
#46674
Merged
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
[Type] Developer Documentation
tyxla
requested review from
Mamaduka,
sgomes,
aristath,
jsnajdr,
swissspidy,
flootr,
chad1008,
ntsekouras and
brookewp
tyxla
requested review from
nerrad,
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Size Change: +38 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Mamaduka
reviewed
Dec 20, 2022
| function removeEntitiesById( entities, ids ) { | ||
| return Object.fromEntries( | ||
| Object.entries( entities ).filter( | ||
| ( [ id ] ) => ! ids.some( ( itemId ) => +itemId === +id ) |
There was a problem hiding this comment.
@tyxla, do you know if omit also casts values internally?
Not every entity ID will be an int. e.g., Templates and template parts use strings for IDs.
wp.data.select('core').getEntityRecords( 'postType', 'wp_template' );
There was a problem hiding this comment.
Yes, it does. It calls toKey on each value:
function toKey(value) {
if (typeof value == 'string' || isSymbol(value)) {
return value;
}
var result = (value + '');
return (result == '0' && (1 / value) == -INFINITY) ? '-0' : result;
}
Thanks, @Mamaduka, I'll have to address your feedback and take strings into consideration there.
There was a problem hiding this comment.
Thanks for the quick follow-up. Would adding the test case for items with string IDs make sense here?
Mamaduka
approved these changes
Dec 20, 2022
|
Thanks, @Mamaduka! I've also tested deleting a navigation menu entity from the Advanced settings of a Navigation block, so I'm pretty confident that this works well:
|
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 Lodash's
_.omit()completely and deprecates the import of that function. There are a few remaining usages in that package 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?
We're creating an inline helper function to reduce repetition. It will omit the unnecessary IDs that are passed as an array, converting the IDs specifically to numbers to ensure that they're compared properly (like Lodash did under the hood when calling
_.omit).I'm also updating docs that used
_.omit()in a couple of examples and deprecating the import of_.omit()in our ESLint config.Testing Instructions
Verify all checks are still green. The changes are covered by unit tests: