Consolidate react-hooks/exhaustive-deps lint rules for O11y #184865
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
dgieselaar
force-pushed
the
observability-eslint-rules-update
branch
from
50be82f
to
7bdf023
Compare
|
/ci |
dgieselaar
force-pushed
the
observability-eslint-rules-update
branch
from
7bdf023
to
8ab5a41
Compare
|
@elasticmachine merge upstream |
botelastic
bot
added
ci:project-deploy-observability
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
dgieselaar
added
the
release_note:skip
| }) => { | ||
| const model = findInventoryModel('host'); | ||
| const model = useMemo(() => findInventoryModel('host'), []); |
There was a problem hiding this comment.
could we move this into useAsync instead?
| decimals: 1, | ||
| subtitle: getSubtitle(options, chart), | ||
| subtitle: getSubtitle ? getSubtitle(chart.value) : getSubtitleFromFormula(chart.value), |
| // 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]); |
There was a problem hiding this comment.
suggestion: dgieselaar#56
| [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] |
There was a problem hiding this comment.
wouldn't it stop complaining if you add successfullyInstalledShipperSetup to the if inside this useFetcher?
There was a problem hiding this comment.
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.
| @@ -24,14 +24,17 @@ export const PolicyLink = ({ name }: { name: string }) => { | |||
|
|
|||
| 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 | |||
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
Canvas Sharable Runtime
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
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
|
@achyutjhunjhunwala you're running a type check 😄 this is a lint rule |
kibanamachine
added
v8.15.0
backport:skip
…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>
Use one react-hooks/exhaustive-deps across our Obs plugins, for consistency reasons.