Skip to content

[chore] Reuse bundled lodash and lodash/fp dependency#217467

Merged
dej611 merged 14 commits intoelastic:mainfrom
dej611:fix/lodash-imports
Apr 17, 2025
Merged

[chore] Reuse bundled lodash and lodash/fp dependency#217467
dej611 merged 14 commits intoelastic:mainfrom
dej611:fix/lodash-imports

Conversation

@dej611
Copy link
Copy Markdown
Contributor

@dej611 dej611 commented Apr 8, 2025

Summary

After #217202 and #217034 this the another attempt with lodash and lodash/fp.

In short:
lodash and lodash/fp have a special webpack treatment as they are imported within the shared bundle.
Now webpack is not smart enough to understand that import camelCase from 'lodash/camelCase'; is still pointing to lodash and it thinks that lodash/camelCase is a different package, de-optimizing the bundling caching system.
So I’ve tweaked the import to make it point to the shared bundle and save few kbs here and there

@dej611 dej611 added Team:Operations Kibana-Operations Team release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Apr 11, 2025
@dej611 dej611 marked this pull request as ready for review April 11, 2025 08:42
@dej611 dej611 requested review from a team as code owners April 11, 2025 08:42
@dej611 dej611 requested a review from a team April 11, 2025 08:42
@dej611 dej611 requested review from a team as code owners April 11, 2025 08:42
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@dej611 dej611 requested review from a team as code owners April 11, 2025 08:42
Copy link
Copy Markdown
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

LGTM for the Threat Hunting Investigations team!

Copy link
Copy Markdown
Contributor

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

ResponseOps changes LGTM

Copy link
Copy Markdown
Contributor

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Core changes are all server-side. LGTM.

Copy link
Copy Markdown
Member

@criamico criamico left a comment

Choose a reason for hiding this comment

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

Fleet code LGTM

@drewdaemon
Copy link
Copy Markdown
Contributor

Have we considered adding a linter rule to keep the imports this way?

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Apr 16, 2025

Have we considered adding a linter rule to keep the imports this way?

Yes, but lodash has already many rules which are easy to get in conflict with, unless you list all the inner methods specifically. To make it even worst, lodash/fp has to list also its internal methods too, making very verbose the thing.
I'll ping operations team, but it probably deserves a separated PR for it.

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
advancedSettings 97 93 -4
apm 1890 1886 -4
cloudSecurityPosture 700 689 -11
dataViewFieldEditor 156 154 -2
fleet 1205 1183 -22
grokdebugger 28 27 -1
infra 1433 1418 -15
observabilityAiAssistantManagement 387 383 -4
observabilityShared 208 204 -4
profiling 290 286 -4
securitySolution 7244 7214 -30
telemetryManagementSection 73 69 -4
threatIntelligence 178 169 -9
total -114

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
advancedSettings 36.8KB 35.2KB -1.6KB
apm 2.6MB 2.6MB -1.5KB
cases 1.3MB 1.3MB -52.0B
cloudSecurityPosture 525.3KB 522.6KB -2.7KB
dataViewFieldEditor 159.7KB 158.6KB -1.1KB
esql 243.3KB 243.2KB -47.0B
fleet 1.7MB 1.7MB -3.6KB
grokdebugger 8.3KB 7.6KB -729.0B
infra 1.2MB 1.2MB -4.8KB
observabilityAiAssistantManagement 92.0KB 90.5KB -1.5KB
profiling 383.9KB 382.3KB -1.5KB
securitySolution 9.0MB 9.0MB -7.1KB
telemetryManagementSection 31.4KB 29.9KB -1.5KB
threatIntelligence 754.2KB 752.3KB -1.9KB
total -29.7KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observabilityShared 86.6KB 85.0KB -1.6KB
securitySolution 88.8KB 88.7KB -77.0B
threatIntelligence 13.0KB 13.0KB -55.0B
total -1.7KB
Unknown metric groups

ESLint disabled line counts

id before after diff
grokdebugger 1 0 -1

Total ESLint disabled count

id before after diff
grokdebugger 1 0 -1

History

Copy link
Copy Markdown
Contributor

@janmonschke janmonschke left a comment

Choose a reason for hiding this comment

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

THI changes lgtm

Copy link
Copy Markdown
Contributor

@kapral18 kapral18 left a comment

Choose a reason for hiding this comment

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

LGTM for explore

Copy link
Copy Markdown
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

Detection Engine changes LGTM.

@dej611 dej611 merged commit e21bec3 into elastic:main Apr 17, 2025
10 checks passed
davismcphee pushed a commit to davismcphee/kibana that referenced this pull request Apr 22, 2025
## Summary

After elastic#217202 and elastic#217034 this the another attempt with `lodash` and
`lodash/fp`.

In short:
`lodash` and `lodash/fp` have a special webpack treatment as they are
imported within the shared bundle.
Now webpack is not smart enough to understand that `import camelCase
from 'lodash/camelCase';` is still pointing to `lodash` and it thinks
that `lodash/camelCase` is a different package, de-optimizing the
bundling caching system.
So I’ve tweaked the import to make it point to the shared bundle and
save few kbs here and there
dej611 added a commit that referenced this pull request Apr 24, 2025
## Summary

Similar to #217034, #217202 and #217467 this time applied to
`react-use`.

This is a slightly different approach than #217034 as we're caching here
only the most common/frequently used methods from the `react-use`
library and leaving the rest to be loaded within the specific plugin
chunks.

What this PR does it fundamentally:
* adds `7.x kb` to the shared bundle
* overall the startup bundle size shrinks about `3.5 kb`
* the async bundle size shrinks of about `350 kb` (mainly due to 3
imports which were targeting `react-use/lib`).

An alternative approach would be to just fix the async import strings in
there, but I thought to it was worth it to make the long step here.
Feedback appreciated.
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
## Summary

After elastic#217202 and elastic#217034 this the another attempt with `lodash` and
`lodash/fp`.

In short:
`lodash` and `lodash/fp` have a special webpack treatment as they are
imported within the shared bundle.
Now webpack is not smart enough to understand that `import camelCase
from 'lodash/camelCase';` is still pointing to `lodash` and it thinks
that `lodash/camelCase` is a different package, de-optimizing the
bundling caching system.
So I’ve tweaked the import to make it point to the shared bundle and
save few kbs here and there
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
)

## Summary

Similar to elastic#217034, elastic#217202 and elastic#217467 this time applied to
`react-use`.

This is a slightly different approach than elastic#217034 as we're caching here
only the most common/frequently used methods from the `react-use`
library and leaving the rest to be loaded within the specific plugin
chunks.

What this PR does it fundamentally:
* adds `7.x kb` to the shared bundle
* overall the startup bundle size shrinks about `3.5 kb`
* the async bundle size shrinks of about `350 kb` (mainly due to 3
imports which were targeting `react-use/lib`).

An alternative approach would be to just fix the async import strings in
there, but I thought to it was worth it to make the long step here.
Feedback appreciated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. Team:Fleet Team label for Observability Data Collection Fleet team Team:Operations Kibana-Operations Team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.