[ESLint rule] prevent useRecoilCallback without a dependency array (#4411)

Co-authored-by: gitstart-twenty <gitstart-twenty@users.noreply.github.com> Co-authored-by: Matheus <matheus_benini@hotmail.com> Co-authored-by: v1b3m <vibenjamin6@gmail.com>
2024-11-25 09:13:22 +03:00 · 2024-03-12 15:12:17 +01:00 · 2024-03-12 15:12:17 +01:00 · 60598bf235
commit 60598bf235
parent 41c7cd8cf7
11 changed files with 174 additions and 52 deletions
--- a/packages/twenty-front/.eslintrc.cjs
+++ b/packages/twenty-front/.eslintrc.cjs
@ -51,6 +51,7 @@ module.exports = {
    '@nx/workspace-component-props-naming': 'error',
    '@nx/workspace-explicit-boolean-predicates-in-if': 'error',
    '@nx/workspace-use-getLoadable-and-getValue-to-get-atoms': 'error',
+    '@nx/workspace-useRecoilCallback-has-dependency-array': 'error',

    'react/no-unescaped-entities': 'off',
    'react/prop-types': 'off',
--- a/packages/twenty-front/src/modules/command-menu/hooks/useCommandMenu.ts
+++ b/packages/twenty-front/src/modules/command-menu/hooks/useCommandMenu.ts
@ -24,10 +24,10 @@ export const useCommandMenu = () => {
    goBackToPreviousHotkeyScope,
  } = usePreviousHotkeyScope();

-  const openCommandMenu = () => {
+  const openCommandMenu = useCallback(() => {
    setIsCommandMenuOpened(true);
    setHotkeyScopeAndMemorizePreviousScope(AppHotkeyScope.CommandMenuOpen);
-  };
+  }, [setHotkeyScopeAndMemorizePreviousScope, setIsCommandMenuOpened]);

  const closeCommandMenu = useRecoilCallback(
    ({ snapshot }) =>
@ -60,6 +60,7 @@ export const useCommandMenu = () => {
          openCommandMenu();
        }
      },
+    [closeCommandMenu, openCommandMenu],
  );

  const addToCommandMenu = useCallback(
--- a/packages/twenty-front/src/modules/object-record/record-action-bar/hooks/useRecordActionBar.tsx
+++ b/packages/twenty-front/src/modules/object-record/record-action-bar/hooks/useRecordActionBar.tsx
@ -45,29 +45,40 @@ export const useRecordActionBar = ({
    objectNameSingular: objectMetadataItem.nameSingular,
  });

-  const handleFavoriteButtonClick = useRecoilCallback(({ snapshot }) => () => {
-    if (selectedRecordIds.length > 1) {
-      return;
-    }
+  const handleFavoriteButtonClick = useRecoilCallback(
+    ({ snapshot }) =>
+      () => {
+        if (selectedRecordIds.length > 1) {
+          return;
+        }

-    const selectedRecordId = selectedRecordIds[0];
-    const selectedRecord = snapshot
-      .getLoadable(recordStoreFamilyState(selectedRecordId))
-      .getValue();
+        const selectedRecordId = selectedRecordIds[0];
+        const selectedRecord = snapshot
+          .getLoadable(recordStoreFamilyState(selectedRecordId))
+          .getValue();

-    const foundFavorite = favorites?.find(
-      (favorite) => favorite.recordId === selectedRecordId,
-    );
+        const foundFavorite = favorites?.find(
+          (favorite) => favorite.recordId === selectedRecordId,
+        );

-    const isFavorite = !!selectedRecordId && !!foundFavorite;
+        const isFavorite = !!selectedRecordId && !!foundFavorite;

-    if (isFavorite) {
-      deleteFavorite(foundFavorite.id);
-    } else if (isDefined(selectedRecord)) {
-      createFavorite(selectedRecord, objectMetadataItem.nameSingular);
-    }
-    callback?.();
-  });
+        if (isFavorite) {
+          deleteFavorite(foundFavorite.id);
+        } else if (isDefined(selectedRecord)) {
+          createFavorite(selectedRecord, objectMetadataItem.nameSingular);
+        }
+        callback?.();
+      },
+    [
+      callback,
+      createFavorite,
+      deleteFavorite,
+      favorites,
+      objectMetadataItem.nameSingular,
+      selectedRecordIds,
+    ],
+  );

  const handleDeleteClick = useCallback(async () => {
    callback?.();
--- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx
+++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx
@ -4,9 +4,11 @@ import { useRecoilCallback, useRecoilValue } from 'recoil';

 import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
 import { useDeleteOneRecord } from '@/object-record/hooks/useDeleteOneRecord';
+import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata';
 import { RecordTable } from '@/object-record/record-table/components/RecordTable';
 import { EntityDeleteContext } from '@/object-record/record-table/contexts/EntityDeleteHookContext';
 import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates';
+import { ColumnDefinition } from '@/object-record/record-table/types/ColumnDefinition';
 import { IconPlus } from '@/ui/display/icon';
 import { Button } from '@/ui/input/button/components/Button';
 import AnimatedPlaceholder from '@/ui/layout/animated-placeholder/components/AnimatedPlaceholder';
@ -92,11 +94,16 @@ export const RecordTableWithWrappers = ({
                <RecordTable
                  recordTableId={recordTableId}
                  objectNameSingular={objectNameSingular}
-                  onColumnsChange={useRecoilCallback(() => (columns) => {
-                    persistViewFields(
-                      mapColumnDefinitionsToViewFields(columns),
-                    );
-                  })}
+                  onColumnsChange={useRecoilCallback(
+                    () => (columns) => {
+                      persistViewFields(
+                        mapColumnDefinitionsToViewFields(
+                          columns as ColumnDefinition<FieldMetadata>[],
+                        ),
+                      );
+                    },
+                    [persistViewFields],
+                  )}
                  createRecord={createRecord}
                />
                <DragSelect
--- a/packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useSetRowSelectedState.ts
+++ b/packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useSetRowSelectedState.ts
@ -5,7 +5,11 @@ import { useRecordTableStates } from '@/object-record/record-table/hooks/interna
 export const useSetRowSelectedState = (recordTableId?: string) => {
  const { isRowSelectedFamilyState } = useRecordTableStates(recordTableId);

-  return useRecoilCallback(({ set }) => (rowId: string, selected: boolean) => {
-    set(isRowSelectedFamilyState(rowId), selected);
-  });
+  return useRecoilCallback(
+    ({ set }) =>
+      (rowId: string, selected: boolean) => {
+        set(isRowSelectedFamilyState(rowId), selected);
+      },
+    [isRowSelectedFamilyState],
+  );
 };
--- a/packages/twenty-front/src/modules/settings/developers/hooks/useGeneratedApiKeys.ts
+++ b/packages/twenty-front/src/modules/settings/developers/hooks/useGeneratedApiKeys.ts
@ -8,5 +8,6 @@ export const useGeneratedApiKeys = () => {
      (apiKeyId: string, apiKey: string | null) => {
        set(generatedApiKeyFamilyState(apiKeyId), apiKey);
      },
+    [],
  );
 };
--- a/packages/twenty-front/src/modules/ui/feedback/snack-bar-manager/hooks/useSnackBar.tsx
+++ b/packages/twenty-front/src/modules/ui/feedback/snack-bar-manager/hooks/useSnackBar.tsx
@ -14,12 +14,16 @@ export const useSnackBar = () => {
    SnackBarManagerScopeInternalContext,
  );

-  const handleSnackBarClose = useRecoilCallback(({ set }) => (id: string) => {
-    set(snackBarInternalScopedState({ scopeId }), (prevState) => ({
-      ...prevState,
-      queue: prevState.queue.filter((snackBar) => snackBar.id !== id),
-    }));
-  });
+  const handleSnackBarClose = useRecoilCallback(
+    ({ set }) =>
+      (id: string) => {
+        set(snackBarInternalScopedState({ scopeId }), (prevState) => ({
+          ...prevState,
+          queue: prevState.queue.filter((snackBar) => snackBar.id !== id),
+        }));
+      },
+    [scopeId],
+  );

  const setSnackBarQueue = useRecoilCallback(
    ({ set }) =>
--- a/packages/twenty-front/src/modules/ui/utilities/scroll/hooks/useListenScroll.ts
+++ b/packages/twenty-front/src/modules/ui/utilities/scroll/hooks/useListenScroll.ts
@ -12,28 +12,40 @@ export const useListenScroll = <T extends Element>({
 }: {
  scrollableRef: React.RefObject<T>;
 }) => {
-  const hideScrollBarsCallback = useRecoilCallback(({ snapshot }) => () => {
-    const isScrolling = snapshot.getLoadable(isScrollingState()).getValue();
+  const hideScrollBarsCallback = useRecoilCallback(
+    ({ snapshot }) =>
+      () => {
+        const isScrolling = snapshot.getLoadable(isScrollingState()).getValue();

-    if (!isScrolling) {
-      scrollableRef.current?.classList.remove('scrolling');
-    }
-  });
+        if (!isScrolling) {
+          scrollableRef.current?.classList.remove('scrolling');
+        }
+      },
+    [scrollableRef],
+  );

-  const handleScrollStart = useRecoilCallback(({ set }) => (event: Event) => {
-    set(isScrollingState(), true);
-    scrollableRef.current?.classList.add('scrolling');
+  const handleScrollStart = useRecoilCallback(
+    ({ set }) =>
+      (event: Event) => {
+        set(isScrollingState(), true);
+        scrollableRef.current?.classList.add('scrolling');

-    const target = event.target as HTMLElement;
+        const target = event.target as HTMLElement;

-    set(scrollTopState(), target.scrollTop);
-    set(scrollLeftState(), target.scrollLeft);
-  });
+        set(scrollTopState(), target.scrollTop);
+        set(scrollLeftState(), target.scrollLeft);
+      },
+    [scrollableRef],
+  );

-  const handleScrollEnd = useRecoilCallback(({ set }) => () => {
-    set(isScrollingState(), false);
-    debounce(hideScrollBarsCallback, 1000)();
-  });
+  const handleScrollEnd = useRecoilCallback(
+    ({ set }) =>
+      () => {
+        set(isScrollingState(), false);
+        debounce(hideScrollBarsCallback, 1000)();
+      },
+    [hideScrollBarsCallback],
+  );

  useEffect(() => {
    const refTarget = scrollableRef.current;
--- a/tools/eslint-rules/index.ts
+++ b/tools/eslint-rules/index.ts
@ -38,6 +38,10 @@ import {
  rule as useGetLoadableAndGetValueToGetAtoms,
  RULE_NAME as useGetLoadableAndGetValueToGetAtomsName,
 } from './rules/use-getLoadable-and-getValue-to-get-atoms';
+import {
+  rule as useRecoilCallbackHasDependencyArray,
+  RULE_NAME as useRecoilCallbackHasDependencyArrayName,
+} from './rules/useRecoilCallback-has-dependency-array';

 /**
 * Import your custom workspace rules at the top of this file.
@ -77,5 +81,7 @@ module.exports = {
    [useGetLoadableAndGetValueToGetAtomsName]:
      useGetLoadableAndGetValueToGetAtoms,
    [maxConstsPerFileName]: maxConstsPerFile,
+    [useRecoilCallbackHasDependencyArrayName]:
+      useRecoilCallbackHasDependencyArray,
  },
 };
--- a/tools/eslint-rules/rules/useRecoilCallback-has-dependency-array.spec.ts
+++ b/tools/eslint-rules/rules/useRecoilCallback-has-dependency-array.spec.ts
@ -0,0 +1,29 @@
+import { TSESLint } from '@typescript-eslint/utils';
+
+import { rule, RULE_NAME } from './useRecoilCallback-has-dependency-array';
+
+const ruleTester = new TSESLint.RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+});
+
+ruleTester.run(RULE_NAME, rule, {
+  valid: [
+    {
+      code: 'const someValue = useRecoilCallback(() => () => {}, []);',
+    },
+    {
+      code: 'const someValue = useRecoilCallback(() => () => {}, [dependency]);',
+    },
+  ],
+  invalid: [
+    {
+      code: 'const someValue = useRecoilCallback(({}) => () => {});',
+      errors: [
+        {
+          messageId: 'isNecessaryDependencyArray',
+        },
+      ],
+      output: 'const someValue = useRecoilCallback(({}) => () => {}, []);',
+    },
+  ],
+});
--- a/tools/eslint-rules/rules/useRecoilCallback-has-dependency-array.ts
+++ b/tools/eslint-rules/rules/useRecoilCallback-has-dependency-array.ts
@ -0,0 +1,46 @@
+import { ESLintUtils } from '@typescript-eslint/utils';
+
+// NOTE: The rule will be available in ESLint configs as "@nx/workspace-useRecoilCallback-has-dependency-array"
+export const RULE_NAME = 'useRecoilCallback-has-dependency-array';
+
+export const rule = ESLintUtils.RuleCreator(() => __filename)({
+  name: RULE_NAME,
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Ensure `useRecoilCallback` is used with a dependency array',
+      recommended: 'recommended',
+    },
+    schema: [],
+    messages: {
+      isNecessaryDependencyArray:
+        'Is necessary dependency array with useRecoilCallback',
+    },
+    fixable: 'code',
+  },
+  defaultOptions: [],
+  create: (context) => {
+    return {
+      CallExpression: (node) => {
+        const { callee } = node;
+        if (
+          callee.type === 'Identifier' &&
+          callee.name === 'useRecoilCallback'
+        ) {
+          const depsArg = node.arguments;
+          if (depsArg.length === 1) {
+            context.report({
+              node: callee,
+              messageId: 'isNecessaryDependencyArray',
+              data: {
+                callee,
+                deps: depsArg[0],
+              },
+              fix: (fixer) => fixer.insertTextAfter(depsArg[0], ', []'),
+            });
+          }
+        }
+      },
+    };
+  },
+});