Skip to content
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

Remove lodash dependency from '@wordpress/element' #16938

Closed
nerrad opened this issue Aug 7, 2019 · 13 comments · Fixed by #42898
Closed

Remove lodash dependency from '@wordpress/element' #16938

nerrad opened this issue Aug 7, 2019 · 13 comments · Fixed by #42898
Assignees
Labels
[Package] Element /packages/element [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@nerrad
Copy link
Contributor

nerrad commented Aug 7, 2019

Is your feature request related to a problem? Please describe.

One of the challenges for projects that want to use various WordPress packages on the frontend is keeping js bundles small. While lodash itself is small in size, the fact that wp.element depends on it means that any code wanting to enqueue wp.element on the frontend is also going to load lodash.

Since there are many native implementations for lodash features that can be used reliably (and in some cases are faster than lodash), I think it'd be good to remove this package's dependency on lodash and replace with native implementation.

Describe the solution you'd like

Replace lodash dependency in @wordpress/element by reimplementing any lodash features as native js.

@nerrad nerrad added [Package] Element /packages/element [Type] Enhancement A suggestion for improvement. labels Aug 7, 2019
@nerrad nerrad self-assigned this Aug 7, 2019
@nerrad
Copy link
Contributor Author

nerrad commented Aug 11, 2019

So, the following lodash functions are relatively trivial to re-implement using native js (for the use-case in wp.element):

  • isArray
  • isNumber
  • isString
  • isEmpty
  • castArray
  • startsWith
  • omit

However, there are a couple that are a bit more involved:

  • kebabCase
  • isPlainObject

Since the last two would involve essentially copying in the same thing that lodash did I'm wondering if the effort here is still worth it.

One alternative I've been pondering when it comes to how WordPress exposes libraries now that javascript is more "first-class" is why we don't take advantage of cdn distributed scripts which to some degree can eliminate the concerns of common dependencies such as lodash, react etc. I'm going to bring this up in the next #core-js meeting.

@adamsilverstein
Copy link
Member

Since the last two would involve essentially copying in the same thing that lodash did I'm wondering if the effort here is still worth it.

One other advantage would be avoiding conflicts with other code on the page using lodash or underscore.

However, there are a couple that are a bit more involved:

kebabCase
isPlainObject

These are pretty tiny, can we just import and bundle them (or copy the code directly)?

https://github.com/lodash/lodash/blob/master/kebabCase.js
https://github.com/lodash/lodash/blob/master/isPlainObject.js

@nerrad
Copy link
Contributor Author

nerrad commented Aug 12, 2019

These are pretty tiny, can we just import and bundle them (or copy the code directly)?

They are tiny, but they have dependencies (follow the internal dependency chain for kebabCase for instance). I'm wondering though, if there are only a few lodash helpers like these two, if we should just use the custom builds feature of lodash and create a special WP package (@wordpress/utils ?) for lodash helpers such as kebabCase etc that have a bit more involved code than what can simply be implemented using native js?

@adamsilverstein
Copy link
Member

worth a try, removing the lodash dependency feels pretty valuable, especially in the WordPress admin context.

@swissspidy
Copy link
Member

Is @wordpress/element the only package depending on lodash or are there other places where we might be able to remove the dependency?

@nerrad
Copy link
Contributor Author

nerrad commented Aug 13, 2019

Is @wordpress/element the only package depending on lodash or are there other places where we might be able to remove the dependency?

It's extensively used in multiple packages in this repository. Ideally it'd be nice to remove it as a dependency everywhere and instead just expose a subset of lodash utils for use (via another package). I'll create another issue for discussing this.

@chrisvanpatten
Copy link
Contributor

One small concern is that as a third party developer, I've used lodash extensively within my own blocks, using the build provided in core. I can understand the appeal of removing a dependency but it feels like a small price to pay for all of the DevEx improvements it brings. Of course I can provide my own bundle if Gutenberg stops using it (or builds a custom build) but then we've just introduced a bunch of duplication as any plugin using lodash may decide to provide its own bundle.

@chrisvanpatten
Copy link
Contributor

(In other words, this feels a bit like seeing the forest for the trees. It's a nice optimization, but what challenges/duplication would it lead to for third-parties?)

@nerrad
Copy link
Contributor Author

nerrad commented Aug 13, 2019

I think for back compat we'd need to keep lodash available as an externally provided script in WP core. So your concern here Chris can be mitigated.

As a flipside to your argument, there is also the challenge of current WordPress packages that have a dependency on lodash unnecessarily which adds additional weight everywhere they are enqueued. This specific pull is an example of that. If someone wants to use wp.element on the frontend (and is not needing any of the lodash functions), lodash is loaded regardless. This is something currently experienced by the WooCommerce blocks team where we don't need lodash functionality in the frontend. This in turn means that for now, we're not using wp.element and just use react and react-dom directly (via what's provided by WP).

Not only that, but I have also experienced issues with third party compatibility on some sites where a plugin or theme a site owner has installed is using lodash on the frontend in one of their bundled scripts directly (not enqueuing what WP provides) which leads to conflicts. It'd be nice to avoid that problem if possible.

@aduth
Copy link
Member

aduth commented Mar 23, 2020

Noting that @ZebulanStanphill has been doing some work lately to convert some usages of Lodash to native equivalents:

And a relevant comment of mine at #21054 (comment), notably this point:

In WordPress, all of Lodash will always be loaded anyways (and it's referenced as an external), so there will be no immediate network bundle size benefit for WordPress as long as Lodash is used anywhere. Therefore, it would mostly be for the benefit of third-party package consumers.

We won't see any immediate benefit in the editor/admin of removing Lodash in individual packages, since the full module is referenced as an external to each package. It would only be at the point if and when the usage of Lodash is so little that inlining Lodash usage to respective packages would be of greater overall benefit. I'm not sure we'd ever get to that point, to be honest. It's true there's plenty of low-hanging fruit with the filter, includes, isFunction, etc. functions, but there's plenty other utilities which provide benefit that aren't easily recreated in native JavaScript. And as noted in earlier comments, the implementation of Lodash is such that it reuses many base implementations. This can be beneficial in some cases, but not in the scenario where we're trying to use only a few methods per each of our packages.

That said, I am sympathetic to a few specific reasons for reducing overall usage:

  • Lodash's additional tolerances are a convenience. But they're also a crutch, and are indicative of types not being well known, where by tolerating them, we are likelier to encounter additional errors elsewhere in the logic.
  • Lodash is bigger than you'd think it is when you're at a point of optimizing performance. For example, this was the result of a bundle analysis I did recently for my Dones project. That's the entire front-end application, and with Lodash configured for tree-shaking.
  • Even if we can't benefit from eliminating modules in Lodash in WordPress itself, removing dependencies from projects will benefit third-party consumers. This is more applicable in some packages more than others, based on whether we'd expect those packages to be used outside the WordPress admin.
    • I hadn't previously given much consideration to usage on the front-end, but it's a good one to highlight as well.
  • Over time, improvements to the browser runtimes may provide performance advantages. The browser could theoretically optimize something like Object.assign in native browserland code moreso than any JavaScript could hope to achieve.
  • The abstractions are often unnecessary, and in some cases may yield confusion or a more difficult developer onboarding. For example, a function like isNull does not bring much value to use when weighing the overhead in considering (a) what it is and (b) how it potentially differs from === null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Element /packages/element [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants