-
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: Refactor away from _.castArray()
#45098
Conversation
Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
Size Change: +25 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
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, the code looks correct 👍
AFAIK, castArray
will accept "array-like" things as well, so the behaviour isn't exactly the same, but I don't think that's going to be a problem here.
I don't think that's the case, or do you have anything specific in mind? AFAIK it's pretty conservative:
which makes sense, because under the hood, it actually uses https://github.com/lodash/lodash/blob/4.17.15/lodash.js#L11000 https://github.com/lodash/lodash/blob/4.17.15/lodash.js#L11286 |
That's good to know, thank you! I was under the impression that |
What?
This PR removes Lodash's
_.castArray()
completely and deprecates the method. 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-fetch
package haslodash
as a dependency #39495How?
We're safely replacing a few usages with a simple native
Array.isArray()
check that manually casts to an array if necessary.Testing Instructions