[security_solution] enable react-hooks/exhaustive-deps #68470

oatkiller · 2020-06-06T12:41:18Z

Summary

Enable react-hooks/exhaustive-deps rule for security_solution
Disable it anywhere that it would catch an issue

Checklist

Delete any items that are not applicable to this PR.

For maintainers

This was checked for breaking API changes and was labeled appropriately

* Enable `react-hooks/exhaustive-deps` rule for security_solution * Disable it anywhere that it would catch an issue

oatkiller · 2020-06-06T12:44:40Z

x-pack/plugins/security_solution/public/alerts/containers/detection_engine/rules/use_rules.tsx

@@ -46,7 +46,7 @@ export const useRules = ({
    let isSubscribed = true;
    const abortCtrl = new AbortController();

-    async function fetchData(forceReload: boolean = false) {
+    async function fetchData() {


removing unused parameters

oatkiller · 2020-06-06T12:44:55Z

x-pack/plugins/security_solution/public/alerts/containers/detection_engine/rules/use_rules.tsx

@@ -80,7 +80,7 @@ export const useRules = ({

    fetchData();
    reFetchRules.current = (refreshPrePackagedRule: boolean = false) => {
-      fetchData(true);
+      fetchData();


removing unused parameters

oatkiller · 2020-06-06T12:45:51Z

x-pack/plugins/security_solution/public/alerts/containers/detection_engine/rules/use_rules.tsx

  }, [
    pagination.page,
    pagination.perPage,
    filterOptions.filter,
    filterOptions.sortField,
    filterOptions.sortOrder,
+    // eslint-disable-next-line react-hooks/exhaustive-deps


Doing a sort on every render is not generally advisable. Even if it happened to be harmless here, when reading this code I feel the urge to confirm that filterOptions.tags is of a constant size.

Might be worth scheduling work to improve. We could probably just wrap it in a useMemo if nothing else.

oatkiller · 2020-06-06T12:49:05Z

x-pack/plugins/security_solution/public/cases/components/create/index.tsx

@@ -91,8 +91,10 @@ export const Create = React.memo(() => {
  const onSubmit = useCallback(async () => {
    const { isValid, data } = await form.submit();
    if (isValid) {
+      // `postCase`'s type is incorrect, it actually returns a promise


Is this comment correct? I think we could update the type.

oatkiller · 2020-06-06T12:52:18Z

x-pack/plugins/security_solution/public/timelines/components/flyout/pane/index.tsx

@@ -50,7 +50,7 @@ const FlyoutPaneComponent: React.FC<FlyoutPaneComponentProps> = ({
  const dispatch = useDispatch();

  const onResizeStop: ResizeCallback = useCallback(
-    (e, direction, ref, delta) => {
+    (_e, _direction, _ref, delta) => {


Tell tsserver/tsc to ignore the fact that these are unused

oatkiller · 2020-06-06T12:55:47Z

x-pack/plugins/security_solution/public/timelines/components/open_timeline/index.tsx

        await deleteTimelines(timelineIds);
      },
      [deleteTimelines]
    );

    /** Invoked when the user clicks the action to delete the selected timelines */
    const onDeleteSelected: OnDeleteSelected = useCallback(async () => {
+      // The type for `deleteTimelines` is incorrect, it returns a Promise


Is this comment right? We could probably fix the type.

oatkiller · 2020-06-06T12:57:35Z

...ck/plugins/security_solution/public/common/components/ml/anomaly/use_anomalies_table_data.ts

    influencersOrCriteriaToString(criteriaFields),
    startDate,
    endDate,
    skip,
    userPermissions,
+    // eslint-disable-next-line react-hooks/exhaustive-deps


It might be worth improving this, as sorting something each render could hinder performance.

oatkiller · 2020-06-06T12:57:58Z

...ty_solution/public/common/components/ml_popover/jobs_table/filters/groups_filter_popover.tsx

@@ -46,6 +46,7 @@ export const GroupsFilterPopoverComponent = ({

  useEffect(() => {
    onSelectedGroupsChanged(selectedGroups);
+    // eslint-disable-next-line react-hooks/exhaustive-deps


Another instance of sorting in render. Thoughts on these?

oatkiller · 2020-06-06T12:58:09Z

...lugins/security_solution/public/network/components/embeddables/map_tool_tip/map_tool_tip.tsx

  }, [
    featureIndex,
+    // eslint-disable-next-line react-hooks/exhaustive-deps


Another sort in render

kibanamachine · 2020-06-07T14:22:51Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5705a93

History

💔 Build #52240 failed 3dd03f1

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

* Enable `react-hooks/exhaustive-deps` rule for security_solution * Disable it anywhere that it would catch an issue # Conflicts: # x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_entries.tsx

* master: [ML] DFAnalytics results: ensure ml result fields are shown in data grid (elastic#68305) [security_solution] enable react-hooks/exhaustive-deps (elastic#68470) Closes elastic#66867 by adding missing, requried API params (elastic#68465) [Telemetry] collect number of visualization saved in the past 7, 30 and 90 days (elastic#67865) [Logs UI] View in context tweaks (elastic#67777) Kibana developer examples landing page (elastic#67049) Bump decompress package version (elastic#68386) fix elastic#66185 (elastic#66186) Bump pdfmake package version (elastic#68395) Unskip embeddables/adding_children suite (elastic#68111) Add embed mode options in the Share UI (elastic#58435) Adding key to avoid react warning (elastic#68491)

* Enable `react-hooks/exhaustive-deps` rule for security_solution * Disable it anywhere that it would catch an issue # Conflicts: # x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_entries.tsx

elasticmachine · 2020-07-16T17:35:13Z

Pinging @elastic/siem (Team:SIEM)

elasticmachine · 2021-09-23T14:33:13Z

Pinging @elastic/security-solution (Team: SecuritySolution)

[security_solution] enable react-hooks/exhaustive-deps

cbfcb18

* Enable `react-hooks/exhaustive-deps` rule for security_solution * Disable it anywhere that it would catch an issue

oatkiller requested a review from patrykkopycinski June 6, 2020 12:41

oatkiller requested review from a team as code owners June 6, 2020 12:41

revert accidental change

ac4383d

oatkiller commented Jun 6, 2020

View reviewed changes

revert accidental change

2ed89bf

oatkiller commented Jun 6, 2020

View reviewed changes

revert accidental change

3dd03f1

oatkiller commented Jun 6, 2020

View reviewed changes

oatkiller added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels Jun 6, 2020

oatkiller mentioned this pull request Jun 7, 2020

[SIEM] Restore rules of hooks #67878

Closed

2 tasks

removed unused type

5705a93

patrykkopycinski approved these changes Jun 8, 2020

View reviewed changes

oatkiller merged commit bc39186 into elastic:master Jun 8, 2020

oatkiller deleted the enable-exhaustive-deps-on-security-solution branch June 8, 2020 14:43

oatkiller mentioned this pull request Jun 8, 2020

[7.x] [security_solution] enable react-hooks/exhaustive-deps (#68470) #68525

Merged

LeeDr added the Team:SIEM label Jul 16, 2020

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[security_solution] enable react-hooks/exhaustive-deps #68470

[security_solution] enable react-hooks/exhaustive-deps #68470

oatkiller commented Jun 6, 2020

oatkiller Jun 6, 2020

oatkiller Jun 6, 2020

oatkiller Jun 6, 2020 •

edited

Loading

oatkiller Jun 6, 2020

oatkiller Jun 6, 2020

oatkiller Jun 6, 2020

oatkiller Jun 6, 2020

oatkiller Jun 6, 2020

oatkiller Jun 6, 2020

kibanamachine commented Jun 7, 2020

elasticmachine commented Jul 16, 2020

elasticmachine commented Sep 23, 2021

[security_solution] enable react-hooks/exhaustive-deps #68470

[security_solution] enable react-hooks/exhaustive-deps #68470

Conversation

oatkiller commented Jun 6, 2020

Summary

Checklist

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oatkiller Jun 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Jun 7, 2020

💚 Build Succeeded

History

elasticmachine commented Jul 16, 2020

elasticmachine commented Sep 23, 2021

oatkiller Jun 6, 2020 •

edited

Loading