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

Core: Replace lodash with es-toolkit #28981

Open
wants to merge 18 commits into
base: next
Choose a base branch
from
Open

Core: Replace lodash with es-toolkit #28981

wants to merge 18 commits into from

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Aug 27, 2024

Closes: #28611

What I did

lodash is hard to tree-shake, it's CJS, and we want to move to less heavy, ESM dependencies.

This replaces lodash for es-toolkit which is mostly a drop-in replacement ♥️

I've only changed the core for now.

before:
Screenshot 2024-08-27 at 16 02 20

after:
Screenshot 2024-08-27 at 16 02 26

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.5 MB 77.5 MB 659 B 1.21 0%
initSize 162 MB 163 MB 276 kB 4.91 0.2%
diffSize 85 MB 85.3 MB 275 kB 18.17 0.3%
buildSize 7.57 MB 7.34 MB -228 kB -17304.71 -3.1%
buildSbAddonsSize 1.66 MB 1.57 MB -96.4 kB -Infinity 🔰-6.2%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 2.34 MB 2.3 MB -45.1 kB -4960.95 -2%
buildSbPreviewSize 352 kB 311 kB -40.8 kB -Infinity 🔰-13.1%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 4.55 MB 4.37 MB -182 kB -20049.26 -4.2%
buildPreviewSize 3.02 MB 2.97 MB -45.6 kB -5159.94 -1.5%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 7.5s 5.6s -1s -920ms -0.99 -34.1%
generateTime 25.3s 20.6s -4s -674ms -0.26 -22.6%
initTime 22.7s 15.8s -6s -881ms -0.28 -43.4%
buildTime 14.4s 9.5s -4s -927ms -1.58 🔰-51.8%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6.3s 7.6s 1.2s 0.58 16.3%
devManagerResponsive 4.2s 5s 798ms 0.99 15.8%
devManagerHeaderVisible 758ms 774ms 16ms -0.36 2.1%
devManagerIndexVisible 800ms 806ms 6ms -0.41 0.7%
devStoryVisibleUncached 1.2s 1.1s -48ms -0.61 -4%
devStoryVisible 799ms 807ms 8ms -0.4 1%
devAutodocsVisible 661ms 850ms 189ms 0.63 22.2%
devMDXVisible 625ms 695ms 70ms -0.29 10.1%
buildManagerHeaderVisible 677ms 789ms 112ms 0.1 14.2%
buildManagerIndexVisible 678ms 796ms 118ms -0.11 14.8%
buildStoryVisible 738ms 826ms 88ms -0.11 10.7%
buildAutodocsVisible 617ms 636ms 19ms -0.85 3%
buildMDXVisible 624ms 663ms 39ms -0.44 5.9%

Greptile Summary

This PR replaces the usage of Lodash with es-toolkit across various files in the Storybook core package, aiming to improve tree-shaking and move towards ESM dependencies.

  • Replaced Lodash imports with es-toolkit equivalents in multiple files, including merge, mapValues, isEqual, and debounce functions
  • Updated code/core/package.json to remove Lodash dependency and add es-toolkit
  • Modified StoryStore.ts to use es-toolkit functions and introduced a new picky function for object property selection
  • Adjusted PreviewWeb.test.ts to use es-toolkit's merge and toMerged functions instead of Lodash
  • Updated parseArgsParam.ts and filterArgTypes.ts to use es-toolkit's isPlainObject and pickBy functions respectively

@ndelangen ndelangen self-assigned this Aug 27, 2024
@ndelangen ndelangen added maintenance User-facing maintenance tasks ci:normal labels Aug 27, 2024
@ndelangen ndelangen changed the title Maintenance: Replace lodash with es-toolkit Core: Replace lodash with es-toolkit Aug 27, 2024
Copy link

nx-cloud bot commented Aug 27, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1a8abcf. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Base automatically changed from norbert/babel-dedup-core to next August 27, 2024 14:56
@ndelangen ndelangen marked this pull request as ready for review September 23, 2024 12:25
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

30 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

@@ -202,7 +202,7 @@ export function filterTabs(panels: Addon_BaseType[], parameters?: Record<string,

if (previewTabs || parametersTabs) {
// deep merge global and local settings
const tabs = merge(previewTabs, parametersTabs);
const tabs = merge(previewTabs || {}, parametersTabs || {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using nullish coalescing operator (??) for conciseness

Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, only a few comments.

code/core/src/manager-api/lib/merge.ts Outdated Show resolved Hide resolved
code/core/template/stories/args.stories.ts Show resolved Hide resolved
code/lib/codemod/package.json Show resolved Hide resolved
@raon0211
Copy link
Contributor

Hello! I’m the maintainer of es-toolkit.

We’ve fixed camelCase to better support emojis and updated the pick function to ignore nonexistent keys in the original object. Could you give our latest version, v1.21.0, a try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:normal maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tracking]: Replace Lodash Usage with Native Functionality or Alternatives
3 participants