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

Import only lodash submodules #2032

Closed
yurishkuro opened this issue Dec 7, 2023 · 3 comments · Fixed by #2041
Closed

Import only lodash submodules #2032

yurishkuro opened this issue Dec 7, 2023 · 3 comments · Fixed by #2041

Comments

@yurishkuro
Copy link
Member

In order to reduce the bundle size, lodash recommends importing submodules, e.g. import _get from 'lodash/get' instead of import { debounce } from 'lodash'. Most of our code is already like that, with the exception of just a handful of files:

$ grx lodash packages/*/src | grep -v 'lodash/'
packages/jaeger-ui/src/components/DependencyGraph/DependencyForceGraph.jsx:18:import { debounce } from 'lodash';
packages/jaeger-ui/src/components/TracePage/TraceStatistics/tableValues.tsx:16:import * as _ from 'lodash';
packages/jaeger-ui/src/components/TracePage/TraceStatistics/generateDropdownValue.tsx:15:import _ from 'lodash';
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx:20:import { sortBy } from 'lodash';

This ticket is to clean up these remaining full imports by replacing them with the needed submodule imports.

It will be interesting to see how much it would affect the bundle size:
image

@professorabhay
Copy link

Hey, @yurishkuro! I would like to work on this.

@anshgoyalevil
Copy link
Member

Can you please tell how did you generated this bundle size visualization.

@yurishkuro
Copy link
Member Author

It's auto generated by our build, stats.html

yurishkuro added a commit that referenced this issue Dec 8, 2023
## Which problem is this PR solving?
- Part of #2032

## Description of the changes
- Port partial changes from #2023

## How was this change tested?
- CI

---------

Signed-off-by: MeenuyD <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Co-authored-by: MeenuyD <[email protected]>
yurishkuro pushed a commit that referenced this issue Dec 12, 2023
## Which problem is this PR solving?
- resolves #2032 

## Description of the changes
- Import only lodash submodules

## How was this change tested?
- locally

Super cool that it reduced the size taken by lodash bundle by 70.59%
(master branch vs this)

Before:

![image](https://github.com/jaegertracing/jaeger-ui/assets/94157520/25bc9f04-c4a7-45c0-b481-0332c09fca3c)
lodash: 773.55 KB

After:

![image](https://github.com/jaegertracing/jaeger-ui/assets/94157520/54bbb26e-9804-4718-beaf-02cb382d0f62)
lodash: 227.43 KB

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Ansh Goyal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants