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

[Feature Request]: Drop per-method lodash dependencies #17731

Closed
1 task done
kubijo opened this issue Oct 14, 2024 · 5 comments · Fixed by #18162
Closed
1 task done

[Feature Request]: Drop per-method lodash dependencies #17731

kubijo opened this issue Oct 14, 2024 · 5 comments · Fixed by #18162
Assignees
Labels
good first issue 👋 Used by GitHub to elevate contribution opportunities needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. package: @carbon/react @carbon/react proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 severity: 1 https://ibm.biz/carbon-severity status: help wanted 👐 type: enhancement 💡

Comments

@kubijo
Copy link
Contributor

kubijo commented Oct 14, 2024

The problem

You are using several single-method lodash package dependencies in your codebase, which is discouraged even by the authors.

The reason I came up to this is that we have an app where size is a pretty big issue and inspection of the built artifact shows duplication of lodash code... (where the per-method ones are also needlessly big).

Image

The solution

  • drop all dependencies where native ECMAScript methods exist
  • consider if other methods could be much simplified by using a simplified and project-shared utility that only uses the aforementioned ES6 methods
  • replace the rest with a tree-shake-able import from (lodash-es)[https://www.npmjs.com/package/lodash-es]… ideally with a very permissive range on lodash version to make it more likely that it will get deduplicated.

They also have some kind of code mod in the linked docs page, but I have not used it, so I cannot vouch for it.

Examples

No response

Application/PAL

No response

Business priority

High Priority = pressing release

Available extra resources

No response

Code of Conduct

Copy link
Contributor

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

@tay1orjones
Copy link
Member

@kubijo Thanks for bringing this up! I agree with all points, though I think we could first migrate existing usages to use lodash-es really easily and then later work on refactoring those to native methods.

I don't think the codemod is what we want because it refactors to the main lodash package:

- import get from 'lodash.get'
+ import get from "lodash/get"

- import mapValues from 'lodash.mapvalues'
+ import mapValues from "lodash/mapValues"

- require('lodash.get')
+ require("lodash/get")

- const sortBy = require('lodash.sortby')
+ const sortBy = require("lodash/sortBy")

Whereas we'll instead be aiming for

- import get from 'lodash.get'
+ import { get } from "lodash-es"

@tay1orjones tay1orjones added proposal: accepted This request has gone through triaging and we are accepting PR's against it. status: help wanted 👐 good first issue 👋 Used by GitHub to elevate contribution opportunities package: @carbon/react @carbon/react needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. and removed status: needs triage 🕵️‍♀️ labels Nov 13, 2024
@tay1orjones tay1orjones moved this from Triage to Later 🧊 in Roadmap Nov 13, 2024
Copy link
Contributor

The Carbon team has accepted this proposal! Our team doesn't have the capacity to work on this now, so we are requesting community contributors. Please see the labels for roles that are needed. If you are willing to help out, comment below and we will get in touch!

@kubijo
Copy link
Contributor Author

kubijo commented Nov 13, 2024

@tay1orjones I see ... yeah, that does not look great ... the only utility I see would be that it primes your code with changes you would then rewrite, but a pretty simple regex-based search&replace does that too...

@kubijo
Copy link
Contributor Author

kubijo commented Nov 28, 2024

I've also bumped into this thing: https://es-toolkit.slash.page/ through https://storybook.js.org/blog/storybook-8-4/#we-cut-bundle-size-in-half

I did not hear about it previously, but it seems pretty cool if all they say is true...

@guidari guidari moved this to 🏗 In Progress in Design System Nov 28, 2024
@guidari guidari moved this from 🏗 In Progress to 🚦 In Review in Design System Dec 2, 2024
@github-project-automation github-project-automation bot moved this from Later 🧊 to Completed 🚢 in Roadmap Dec 16, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Community Workgroups Dec 16, 2024
@github-project-automation github-project-automation bot moved this from 🚦 In Review to ✅ Done in Design System Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue 👋 Used by GitHub to elevate contribution opportunities needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. package: @carbon/react @carbon/react proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 severity: 1 https://ibm.biz/carbon-severity status: help wanted 👐 type: enhancement 💡
Projects
Status: Done
Status: ✅ Done
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants