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.

Sign up for GitHub

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

Jump to bottom

Optimize react-query dependencies #144206

Merged
merged 10 commits into from
Oct 31, 2022
Merged

Optimize react-query dependencies #144206

merged 10 commits into from
Oct 31, 2022

Conversation

patrykkopycinski
Copy link
Contributor

@patrykkopycinski patrykkopycinski commented Oct 29, 2022 •
edited
Loading

Summary

Follow up for #139986 where I've noticed a spike in the module counts.
I've noticed increased adoption of react-query across multiple teams, so I think it does make sense to move it to the shared deps

@elastic/kibana-operations I've noticed that @kbn/es-query is used across many plugins, do you think it's a good idea to include that into the shared deps as well?

@patrykkopycinski patrykkopycinski changed the title Chore/optimize react query deps Optimize react-query dependencies Oct 29, 2022
@patrykkopycinski patrykkopycinski marked this pull request as ready for review October 29, 2022 21:44
@patrykkopycinski patrykkopycinski requested a review from a team as a code owner October 29, 2022 21:44
@patrykkopycinski patrykkopycinski self-assigned this Oct 29, 2022
@patrykkopycinski patrykkopycinski added Team:Operations Team label for Operations Team release_note:skip Skip the PR/issue when compiling release notes v8.6.0 labels Oct 29, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 505 441 -64
apm 1339 1275 -64
canvas 1162 1098 -64
cases 543 504 -39
cloudSecurityPosture 228 125 -103
controls 216 152 -64
dashboard 393 329 -64
dashboardEnhanced 96 32 -64
data 521 457 -64
dataViewManagement 183 119 -64
dataVisualizer 380 316 -64
discover 512 448 -64
discoverEnhanced 74 10 -64
fleet 859 729 -130
graph 289 225 -64
infra 1014 950 -64
inputControlVis 109 45 -64
kubernetesSecurity 88 49 -39
lens 959 895 -64
lists 320 256 -64
maps 1008 944 -64
ml 1629 1565 -64
monitoring 510 446 -64
observability 475 411 -64
osquery 380 250 -130
securitySolution 3267 3164 -103
sessionView 140 101 -39
stackAlerts 129 65 -64
synthetics 1022 958 -64
threatIntelligence 266 163 -103
timelines 255 191 -64
transform 330 266 -64
triggersActionsUi 511 447 -64
unifiedFieldList 96 32 -64
unifiedSearch 234 170 -64
upgradeAssistant 200 136 -64
visTypeTimelion 109 45 -64
visTypeVega 232 168 -64
visualizations 374 310 -64
total -2670

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/ui-shared-deps-src 32 35 +3

Async chunks

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

id before after diff
aiops 766.9KB 732.4KB -34.5KB
apm 3.1MB 3.1MB -27.8KB
canvas 1.0MB 1.0MB -26.3KB
cases 386.6KB 334.3KB -52.3KB
cloudSecurityPosture 217.1KB 124.8KB -92.3KB
controls 493.2KB 471.6KB -21.6KB
dashboard 423.7KB 379.3KB -44.4KB
data 52.6KB 52.6KB -5.0B
dataViewManagement 149.2KB 114.6KB -34.7KB
dataVisualizer 571.7KB 537.0KB -34.7KB
discover 461.0KB 416.6KB -44.4KB
discoverEnhanced 44.7KB 0.0B -44.7KB
enterpriseSearch 1.7MB 1.7MB +1.0B
fleet 994.5KB 851.1KB -143.4KB
graph 454.5KB 426.6KB -27.9KB
infra 1011.3KB 970.7KB -40.7KB
inputControlVis 78.3KB 49.7KB -28.6KB
kubernetesSecurity 36.8KB 36.8KB -8.0B
lens 1.3MB 1.3MB -44.1KB
lists 192.5KB 165.7KB -26.8KB
maps 2.7MB 2.7MB +402.0B
ml 3.4MB 3.3MB -34.5KB
monitoring 479.8KB 451.8KB -28.0KB
observability 520.7KB 485.0KB -35.6KB
osquery 1.1MB 1008.7KB -86.9KB
securitySolution 9.6MB 9.5MB -91.4KB
sessionView 379.2KB 379.1KB -48.0B
stackAlerts 73.9KB 73.9KB +28.0B
synthetics 1.0MB 1018.7KB -27.9KB
threatIntelligence 130.2KB 40.1KB -90.1KB
timelines 74.0KB 74.0KB -5.0B
transform 389.0KB 354.4KB -34.7KB
triggersActionsUi 676.8KB 648.8KB -28.0KB
unifiedFieldList 55.2KB 20.4KB -34.8KB
unifiedSearch 260.5KB 216.2KB -44.3KB
upgradeAssistant 179.9KB 152.6KB -27.3KB
visTypeTimelion 109.3KB 74.5KB -34.7KB
visTypeVega 1.7MB 1.6MB -35.7KB
visualizations 269.6KB 246.6KB -22.9KB
total -1.4MB

Page load bundle

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

id before after diff
aiops 5.1KB 5.1KB -4.0B
apm 29.0KB 29.1KB +54.0B
canvas 13.2KB 13.3KB +39.0B
cases 126.2KB 126.2KB +25.0B
cloudSecurityPosture 10.9KB 10.8KB -56.0B
controls 20.6KB 20.6KB -76.0B
dashboard 41.4KB 41.3KB -69.0B
dashboardEnhanced 42.8KB 15.2KB -27.7KB
data 439.1KB 401.0KB -38.1KB
dataViewManagement 4.6KB 4.5KB -54.0B
dataVisualizer 20.4KB 20.4KB -4.0B
discover 26.5KB 26.5KB -11.0B
discoverEnhanced 6.7KB 5.9KB -823.0B
fleet 114.0KB 114.1KB +112.0B
graph 7.3KB 7.3KB -4.0B
infra 84.8KB 84.8KB -46.0B
inputControlVis 5.5KB 5.5KB -54.0B
kbnUiSharedDeps-npmDll 5.0MB 5.1MB +55.6KB
kbnUiSharedDeps-srcJs 3.8MB 3.8MB +44.3KB
kubernetesSecurity 58.9KB 3.7KB -55.1KB
lens 29.2KB 29.3KB +54.0B
lists 3.7KB 3.7KB -4.0B
maps 82.2KB 53.7KB -28.5KB
ml 41.8KB 41.8KB +54.0B
monitoring 23.8KB 23.9KB +54.0B
observability 68.2KB 68.2KB -12.0B
osquery 103.3KB 48.3KB -55.1KB
securitySolution 50.6KB 50.7KB +108.0B
sessionView 60.6KB 5.4KB -55.1KB
stackAlerts 42.4KB 14.2KB -28.2KB
synthetics 24.4KB 24.4KB -4.0B
threatIntelligence 22.6KB 22.6KB +57.0B
timelines 172.8KB 138.1KB -34.7KB
transform 15.3KB 15.3KB +4.0B
triggersActionsUi 100.5KB 100.5KB -4.0B
unifiedFieldList 15.9KB 15.8KB -54.0B
unifiedSearch 49.6KB 49.7KB +13.0B
upgradeAssistant 19.3KB 19.3KB +4.0B
visTypeTimelion 10.6KB 10.6KB +54.0B
visTypeVega 33.6KB 33.6KB -4.0B
visualizations 53.8KB 53.7KB -41.0B
total -223.3KB
Unknown metric groups

API count

id before after diff
@kbn/ui-shared-deps-src 41 44 +3

async chunk count

id before after diff
canvas 19 18 -1
cases 17 16 -1
dashboard 8 7 -1
discover 10 9 -1
discoverEnhanced 1 0 -1
observability 17 16 -1
unifiedSearch 15 14 -1
visualizations 14 13 -1
total -8

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 57 63 +6
osquery 103 108 +5
securitySolution 439 443 +4
total +17

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 65 71 +6
osquery 104 110 +6
securitySolution 516 520 +4
total +18

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @patrykkopycinski

jbudz
jbudz approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

I'm okay with es-query. In theory it probably should be async for every plugin, but given the sync bundle improvements that's probably not the case ATM. If we get to that point we can revert as needed.

@patrykkopycinski patrykkopycinski merged commit fda6b27 into elastic:main Oct 31, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 31, 2022
@patrykkopycinski patrykkopycinski deleted the chore/optimize-react-query-deps branch October 31, 2022 20:47
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 1, 2022
@jloleysens
Merge branch 'main' into files-record-created-by
403b47d 
* main: (43 commits)
  [Synthetics] Step details page screenshot (elastic#143452)
  [Lens] Datatable expression types improvement. (elastic#144173)
  [packages/kbn-journeys] start apm after browser start and stop after browser is closed (elastic#144267)
  [Files] Make files namespace agnostic (elastic#144019)
  Implement base browser-side logging system (elastic#144107)
  Correct wrong multiplier for byte conversion (elastic#143751)
  [Monaco] Add JSON syntax support to the Monaco editor (elastic#143739)
  CCS Smoke Test for Remote Clusters and Index Management  (elastic#142423)
  [api-docs] Daily api_docs build (elastic#144294)
  chore(NA): include progress on Bazel tasks (elastic#144275)
  [RAM] Allow users to see event logs from all spaces they have access to (elastic#140449)
  [APM] Show recommended minimum size when going below 5 minutes (elastic#144170)
  [typecheck] delete temporary target_types dirs in packages (elastic#144271)
  [Security Solution][Endpoint] adds new alert loading utility and un-skip FTR test for endpoint (elastic#144133)
  [performance/journeys] revert data_stress_test_lens.ts journey step (elastic#144261)
  [TIP] Use search strategies in Threat Intelligence (elastic#143267)
  Optimize react-query dependencies (elastic#144206)
  [babel/node] invalidate cache when synth pkg map is updated (elastic#144258)
  [APM] AWS lambda estimated cost (elastic#143986)
  [Maps] layer group wizard (elastic#144129)
  ...
@kpollich
Copy link
Member

kpollich commented Dec 2, 2022 •
edited
Loading

Hi @patrykkopycinski and @jbudz, I've been working on migrating parts of the Fleet codebase to @tanstack/query and I've run into a snag with react-query-devtools. It seems based on the changes in this PR, we're loading a production build of the dev tools package, which results in null coming from the package, rather than the actual dev tools toggle/panel.

react-query-devtools intentionally provides a noop in production builds to reduce bundle size, as it's intended to be a development package only. I think the way we're resolving it as part of the kbn-shared-deps workflow is providing NODE_ENV = production by default and thus grabbing the noop instead of the actual package.

image

image

Wondering if I should be providing NODE_ENV = development when running yarn kbn bootstrap or anything like that in order to use these dev tools? Curious if there's something I'm missing here. Apologies for necroing this PR but it seems the issue w/ dev tools stems from these changes, as we previously had them working in Fleet's debug UI prior to this PR landing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbudz jbudz jbudz approved these changes

Assignees

@patrykkopycinski patrykkopycinski

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@patrykkopycinski @kibana-ci @kpollich @jbudz @kibanamachine