Consolidate react-hooks/exhaustive-deps lint rules for O11y by dgieselaar · Pull Request #184865 · elastic/kibana

dgieselaar · 2024-06-05T18:33:27Z

Use one react-hooks/exhaustive-deps across our Obs plugins, for consistency reasons.

ghost · 2024-06-05T18:33:41Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

dgieselaar · 2024-06-05T18:35:19Z

/ci

dgieselaar · 2024-06-06T09:11:58Z

@elasticmachine merge upstream

elasticmachine · 2024-06-06T09:12:12Z

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

crespocarlos

LGTM. Just a few questions.

crespocarlos · 2024-06-06T10:15:36Z

...bservability_solution/infra/public/components/asset_details/hooks/use_host_metrics_charts.ts

+  overview?: boolean;
 }) => {
-  const model = findInventoryModel('host');
+  const model = useMemo(() => findInventoryModel('host'), []);


could we move this into useAsync instead?

crespocarlos · 2024-06-06T10:18:32Z

...bservability_solution/infra/public/components/asset_details/hooks/use_host_metrics_charts.ts

+      seriesColor,
      decimals: 1,
-      subtitle: getSubtitle(options, chart),
+      subtitle: getSubtitle ? getSubtitle(chart.value) : getSubtitleFromFormula(chart.value),


crespocarlos · 2024-06-06T12:19:33Z

x-pack/plugins/observability_solution/infra/public/hooks/use_lens_attributes.ts

+    // FIXME: this should be memoized upstream, but Dario
+    // cannot find a reasonable fix, so he'll just leave
+    // this in place.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [params.chartType, params.dataset, dataViews, lens]);


suggestion: dgieselaar#56

thanks for this!

crespocarlos · 2024-06-06T12:21:37Z

...ability_onboarding/public/application/quickstart_flows/system_logs/install_elastic_agent.tsx

-    [apiKeyEncoded, onboardingId, installShipperSetupStatus === FETCH_STATUS.SUCCESS]
+    // FIXME: Dario could not find a reasonable fix for successfullyInstalledShipperSetup
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [apiKeyEncoded, onboardingId, successfullyInstalledShipperSetup]


wouldn't it stop complaining if you add successfullyInstalledShipperSetup to the if inside this useFetcher?

nah, it will then complain that it's not in the list of dependencies - it's a behavioral change as well which I would like to avoid to some extent.

crespocarlos · 2024-06-06T12:23:59Z

...observability_solution/synthetics/public/apps/synthetics/components/settings/policy_link.tsx

  const { data } = useFetcher(async () => {
    return ilmLocator?.getLocation({ page: 'policy_edit', policyName: name });
+    // FIXME: Dario thinks there is a better way to do this but
+    // he's getting tired and maybe the Synthetics folks can fix it


kibana-ci · 2024-06-06T13:24:48Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 756917b
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-184865-756917bf3368
Observability Deployment

Failed CI Steps

Metrics [docs]

Async chunks

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

id	before	after	diff
`infra`	1.5MB	1.5MB	-510.0B
`observabilityAIAssistantApp`	152.8KB	152.8KB	+20.0B
`observabilityOnboarding`	198.7KB	198.7KB	+8.0B
`synthetics`	1.0MB	1.0MB	+169.0B
total			-313.0B

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	-	5499	+5499
`total size`	-	8.9MB	+8.9MB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`observabilityOnboarding`	9	14	+5
`observabilityShared`	14	15	+1
`synthetics`	72	84	+12
`uptime`	37	45	+8
total			+26

Total ESLint disabled count

id	before	after	diff
`observabilityOnboarding`	13	18	+5
`observabilityShared`	14	15	+1
`synthetics`	79	91	+12
`uptime`	41	49	+8
total			+26

History

💛 Build #214381 was flaky dc35666
💔 Build #214303 failed 8ab5a41
💔 Build #214249 failed 7bdf023b7a2139f85770513699ff015e3ef4fce8

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

achyutjhunjhunwala

LGTM, Though

I tried removing all your comments in the file x-pack/plugins/observability_solution/observability_onboarding/public/application/quickstart_flows/custom_logs/install_elastic_agent.tsx. and ran

node scripts/type_check.js --project /Users/achyutjhunjhunwala/Workspace/tug_kibana/kibana/x-pack/plugins/observability_solution/observability_onboarding/tsconfig.json

It gave no errors

dgieselaar · 2024-06-07T14:13:28Z

@achyutjhunjhunwala you're running a type check 😄 this is a lint rule

flash1293

LGTM

…184865) Use one react-hooks/exhaustive-deps across our Obs plugins, for consistency reasons. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>

dgieselaar force-pushed the observability-eslint-rules-update branch from 50be82f to 7bdf023 Compare June 5, 2024 18:35

Consolidate react-hooks/exhaustive-deps lint rules for O11y

8ab5a41

dgieselaar force-pushed the observability-eslint-rules-update branch from 7bdf023 to 8ab5a41 Compare June 5, 2024 21:17

dgieselaar marked this pull request as ready for review June 5, 2024 21:19

dgieselaar requested a review from a team June 5, 2024 21:19

dgieselaar requested review from a team as code owners June 5, 2024 21:19

dgieselaar requested a review from a team June 5, 2024 21:19

dgieselaar requested a review from a team as a code owner June 5, 2024 21:19

smith added the apm:review label Jun 5, 2024

Merge branch 'main' into observability-eslint-rules-update

dc35666

botelastic bot added ci:project-deploy-observability Create an Observability project Team:Obs AI Assistant Observability AI Assistant Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. labels Jun 6, 2024

dgieselaar added the release_note:skip Skip the PR/issue when compiling release notes label Jun 6, 2024

Fix lens attributes passed via props (#56)

756917b

crespocarlos approved these changes Jun 6, 2024

View reviewed changes

sorenlouv approved these changes Jun 6, 2024

View reviewed changes

achyutjhunjhunwala approved these changes Jun 7, 2024

View reviewed changes

flash1293 approved these changes Jun 7, 2024

View reviewed changes

dgieselaar merged commit 5381dd7 into elastic:main Jun 8, 2024

kibanamachine added v8.15.0 backport:skip This PR does not require backporting labels Jun 8, 2024

Conversation

dgieselaar commented Jun 5, 2024

Uh oh!

ghost commented Jun 5, 2024

🤖 GitHub comments

Uh oh!

dgieselaar commented Jun 5, 2024

Uh oh!

dgieselaar commented Jun 6, 2024

Uh oh!

elasticmachine commented Jun 6, 2024

Uh oh!

crespocarlos left a comment

Choose a reason for hiding this comment

Uh oh!

crespocarlos Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

crespocarlos Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

crespocarlos Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

dgieselaar Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

crespocarlos Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

dgieselaar Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

crespocarlos Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Jun 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Canvas Sharable Runtime

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

achyutjhunjhunwala left a comment

Choose a reason for hiding this comment

Uh oh!

dgieselaar commented Jun 7, 2024

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

kibana-ci commented Jun 6, 2024 •

edited

Loading