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

Tech: migrate to es version of lodash #20899

Closed
wants to merge 3 commits into from
Closed

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Feb 2, 2023

migrating to es version of lodash (manager)

@ndelangen ndelangen self-assigned this Feb 2, 2023
@ndelangen ndelangen added the maintenance User-facing maintenance tasks label Feb 2, 2023
@valentinpalkovic
Copy link
Contributor

valentinpalkovic commented Feb 2, 2023

@ndelangen Could you quickly compare the bundle sizes and find out whether tree-shaking really works? As mentioned, with our current tsup config tree-shaking might be broken.

@ndelangen
Copy link
Member Author

that seems important, but not really super related to this PR? @valentinpalkovic

@ndelangen ndelangen changed the title Tech: migrate to es version of lodash Tech: migrate to es version of lodash Feb 2, 2023
@valentinpalkovic
Copy link
Contributor

I would say it is important, in a sense, that we potentially increase the bundle size of the manager quite a lot. But if we do not care that much about it, I would agree with you, that my comment is unrelated to this PR.

@valentinpalkovic
Copy link
Contributor

valentinpalkovic commented Feb 2, 2023

So maybe we should first make sure that tsup tree-shakes properly (maybe it already does) and then switch out lodash and other packages by their ES counterparts and adjust the imports.

@IanVS
Copy link
Member

IanVS commented Feb 2, 2023

I agree with @valentinpalkovic. I think we've been working hard to bring down the installation size of storybook, so blowing it up with all of lodash would be a step backwards, if it's not tree-shaking properly.

@ndelangen
Copy link
Member Author

ndelangen commented Feb 3, 2023

Ok, I'll investigate

before:
#20908

after:
#20910

@ndelangen ndelangen closed this Feb 4, 2023
@stof stof deleted the norbert/lodash-es branch August 2, 2023 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants