TWNTY-2244 - ESLint rule: enforce usage of .getLoadable() + .getValue() to get atoms (#4143)

* ESLint rule: enforce usage of .getLoadable() + .getValue() to get atoms

Co-authored-by: Matheus <matheus_benini@hotmail.com>

* Merge main

Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: Matheus <matheus_benini@hotmail.com>

* Fix

* Refactor according to review

Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: Matheus <matheus_benini@hotmail.com>

* Fix linter issue

Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: Matheus <matheus_benini@hotmail.com>

* Fix linter

Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: Matheus <matheus_benini@hotmail.com>

---------

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>
Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>

.vscode/settings.json

@@ -47,5 +47,6 @@
   },
   "search.exclude": {
     "**/.yarn": true,
-  }
+  },
+  "eslint.debug": true
 }

packages/twenty-front/.eslintrc.cjs

@@ -48,6 +48,7 @@ module.exports = {
     '@nx/workspace-styled-components-prefixed-with-styled': 'error',
     '@nx/workspace-no-state-useref': 'error',
     '@nx/workspace-component-props-naming': 'error',
+    '@nx/workspace-use-getLoadable-and-getValue-to-get-atoms': 'error',
 
     'react/no-unescaped-entities': 'off',
     'react/prop-types': 'off',

packages/twenty-front/src/modules/object-metadata/hooks/useGetRelationMetadata.ts

@@ -46,7 +46,7 @@ export const useGetRelationMetadata = () =>
               objectNameType: 'singular',
             }),
           )
-          .valueOrThrow();
+          .getValue();
 
         if (!relationObjectMetadataItem) return null;
 

packages/twenty-front/src/modules/object-record/record-board-deprecated/hooks/internal/useRecordBoardDeprecatedCardFieldsInternal.ts

@@ -35,7 +35,7 @@ export const useRecordBoardDeprecatedCardFieldsInternal = (
       async (
         field: Omit<ColumnDefinition<FieldMetadata>, 'size' | 'position'>,
       ) => {
-        const existingFields = await snapshot
+        const existingFields = snapshot
           .getLoadable(recordBoardCardFieldsScopedState({ scopeId }))
           .getValue();
 

packages/twenty-front/src/modules/object-record/record-board-deprecated/hooks/internal/useRemoveRecordBoardDeprecatedCardIdsInternal.ts

@@ -11,16 +11,14 @@ export const useRemoveRecordBoardDeprecatedCardIdsInternal = () => {
   return useRecoilCallback(
     ({ snapshot, set }) =>
       (cardIdToRemove: string[]) => {
-        const boardColumns = snapshot
+        const boardColumns = snapshot.getLoadable(boardColumnsState).getValue();
-          .getLoadable(boardColumnsState)
-          .valueOrThrow();
 
         boardColumns.forEach((boardColumn) => {
           const columnCardIds = snapshot
             .getLoadable(
               recordBoardCardIdsByColumnIdFamilyState(boardColumn.id),
             )
-            .valueOrThrow();
+            .getValue();
           set(
             recordBoardCardIdsByColumnIdFamilyState(boardColumn.id),
             columnCardIds.filter((cardId) => !cardIdToRemove.includes(cardId)),

packages/twenty-front/src/modules/object-record/record-board-deprecated/hooks/internal/useSetRecordBoardDeprecatedCardSelectedInternal.ts

@@ -19,7 +19,9 @@ export const useSetRecordBoardDeprecatedCardSelectedInternal = (props: any) => {
   const setCardSelected = useRecoilCallback(
     ({ set, snapshot }) =>
       (cardId: string, selected: boolean) => {
-        const activeCardIds = snapshot.getLoadable(activeCardIdsState).contents;
+        const activeCardIds = snapshot
+          .getLoadable(activeCardIdsState)
+          .getValue();
 
         set(isRecordBoardDeprecatedCardSelectedFamilyState(cardId), selected);
         set(actionBarOpenState, selected || activeCardIds.length > 0);
@@ -39,7 +41,9 @@ export const useSetRecordBoardDeprecatedCardSelectedInternal = (props: any) => {
   const unselectAllActiveCards = useRecoilCallback(
     ({ set, snapshot }) =>
       () => {
-        const activeCardIds = snapshot.getLoadable(activeCardIdsState).contents;
+        const activeCardIds = snapshot
+          .getLoadable(activeCardIdsState)
+          .getValue();
 
         activeCardIds.forEach((cardId: string) => {
           set(isRecordBoardDeprecatedCardSelectedFamilyState(cardId), false);

packages/twenty-front/src/modules/object-record/record-board-deprecated/hooks/internal/useUpdateCompanyBoardColumnsInternal.ts

@@ -68,7 +68,7 @@ export const useUpdateCompanyBoardColumnsInternal = () => {
         for (const [id, companyProgress] of Object.entries(companyBoardIndex)) {
           const currentCompanyProgress = snapshot
             .getLoadable(companyProgressesFamilyState(id))
-            .valueOrThrow();
+            .getValue();
 
           if (!isDeeplyEqual(currentCompanyProgress, companyProgress)) {
             set(companyProgressesFamilyState(id), companyProgress);
@@ -78,11 +78,11 @@ export const useUpdateCompanyBoardColumnsInternal = () => {
 
         const currentPipelineSteps = snapshot
           .getLoadable(currentPipelineStepsState)
-          .valueOrThrow();
+          .getValue();
 
         const currentBoardColumns = snapshot
           .getLoadable(boardColumnsState)
-          .valueOrThrow();
+          .getValue();
 
         if (!isDeeplyEqual(pipelineSteps, currentPipelineSteps)) {
           set(currentPipelineStepsState, pipelineSteps);
@@ -133,7 +133,7 @@ export const useUpdateCompanyBoardColumnsInternal = () => {
             .getLoadable(
               recordBoardCardIdsByColumnIdFamilyState(boardColumn.id),
             )
-            .valueOrThrow();
+            .getValue();
 
           if (!isDeeplyEqual(currentBoardCardIds, boardCardIds)) {
             set(

packages/twenty-front/src/modules/object-record/record-store/hooks/useSetRecordInStore.ts

@@ -10,7 +10,7 @@ export const useSetRecordInStore = () => {
         records.forEach((record) => {
           const currentRecord = snapshot
             .getLoadable(recordStoreFamilyState(record.id))
-            .valueOrThrow();
+            .getValue();
 
           if (JSON.stringify(currentRecord) !== JSON.stringify(record)) {
             set(recordStoreFamilyState(record.id), record);

packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useLeaveTableFocus.ts

@@ -26,7 +26,7 @@ export const useLeaveTableFocus = (recordTableId?: string) => {
 
         const currentHotkeyScope = snapshot
           .getLoadable(currentHotkeyScopeState)
-          .valueOrThrow();
+          .getValue();
 
         if (!isSoftFocusActive) {
           return;

packages/twenty-front/src/modules/object-record/record-table/hooks/internal/useSetRecordTableData.ts

@@ -24,7 +24,7 @@ export const useSetRecordTableData = ({
           // TODO: refactor with scoped state later
           const currentEntity = snapshot
             .getLoadable(recordStoreFamilyState(entity.id))
-            .valueOrThrow();
+            .getValue();
 
           if (JSON.stringify(currentEntity) !== JSON.stringify(entity)) {
             set(recordStoreFamilyState(entity.id), entity);

packages/twenty-front/src/modules/pipeline/hooks/usePipelineSteps.ts

@@ -19,8 +19,10 @@ export const usePipelineSteps = () => {
 
   const handlePipelineStepAdd = useRecoilCallback(
     ({ snapshot }) =>
-      async (boardColumn: BoardColumnDefinition) => {
+      (boardColumn: BoardColumnDefinition) => {
-        const currentPipeline = await snapshot.getPromise(currentPipelineState);
+        const currentPipeline = snapshot
+          .getLoadable(currentPipelineState)
+          .getValue();
         if (!currentPipeline?.id) return;
 
         return createOnePipelineStep?.({
@@ -35,8 +37,10 @@ export const usePipelineSteps = () => {
 
   const handlePipelineStepDelete = useRecoilCallback(
     ({ snapshot }) =>
-      async (boardColumnId: string) => {
+      (boardColumnId: string) => {
-        const currentPipeline = await snapshot.getPromise(currentPipelineState);
+        const currentPipeline = snapshot
+          .getLoadable(currentPipelineState)
+          .getValue();
         if (!currentPipeline?.id) return;
 
         return deleteOnePipelineStep?.(boardColumnId);

packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/usePreviousHotkeyScope.ts

@@ -14,7 +14,7 @@ export const usePreviousHotkeyScope = () => {
       () => {
         const previousHotkeyScope = snapshot
           .getLoadable(previousHotkeyScopeState)
-          .valueOrThrow();
+          .getValue();
 
         if (!previousHotkeyScope) {
           return;
@@ -35,7 +35,7 @@ export const usePreviousHotkeyScope = () => {
       (scope: string, customScopes?: CustomHotkeyScopes) => {
         const currentHotkeyScope = snapshot
           .getLoadable(currentHotkeyScopeState)
-          .valueOrThrow();
+          .getValue();
 
         setHotkeyScope(scope, customScopes);
         set(previousHotkeyScopeState, currentHotkeyScope);

packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useScopedHotkeyCallback.ts

@@ -25,7 +25,7 @@ export const useScopedHotkeyCallback = () =>
       }) => {
         const currentHotkeyScopes = snapshot
           .getLoadable(internalHotkeysEnabledScopesState)
-          .valueOrThrow();
+          .getValue();
 
         if (!currentHotkeyScopes.includes(scope)) {
           if (DEBUG_HOTKEY_SCOPE) {

packages/twenty-front/src/modules/ui/utilities/hotkey/hooks/useSetHotkeyScope.ts

@@ -27,7 +27,7 @@ export const useSetHotkeyScope = () =>
       async (hotkeyScopeToSet: string, customScopes?: CustomHotkeyScopes) => {
         const currentHotkeyScope = snapshot
           .getLoadable(currentHotkeyScopeState)
-          .valueOrThrow();
+          .getValue();
 
         if (currentHotkeyScope.scope === hotkeyScopeToSet) {
           if (!isNonNullable(customScopes)) {

tools/eslint-rules/index.ts

@@ -30,6 +30,11 @@ import {
   rule as styledComponentsPrefixedWithStyled,
   RULE_NAME as styledComponentsPrefixedWithStyledName,
 } from './rules/styled-components-prefixed-with-styled';
+import {
+  rule as useGetLoadableAndGetValueToGetAtoms,
+  RULE_NAME as useGetLoadableAndGetValueToGetAtomsName,
+} from './rules/use-getLoadable-and-getValue-to-get-atoms';
+
 /**
  * Import your custom workspace rules at the top of this file.
  *
@@ -64,6 +69,8 @@ module.exports = {
     [sortCssPropertiesAlphabeticallyName]: sortCssPropertiesAlphabetically,
     [styledComponentsPrefixedWithStyledName]:
       styledComponentsPrefixedWithStyled,
+    [useGetLoadableAndGetValueToGetAtomsName]:
+      useGetLoadableAndGetValueToGetAtoms,
     [maxConstsPerFileName]: maxConstsPerFile,
   },
 };

tools/eslint-rules/rules/use-getLoadable-and-getValue-to-get-atoms.spec.ts

@@ -0,0 +1,53 @@
+import { TSESLint } from '@typescript-eslint/utils';
+
+import { rule, RULE_NAME } from './use-getLoadable-and-getValue-to-get-atoms';
+
+const ruleTester = new TSESLint.RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+  parserOptions: {
+    ecmaFeatures: {
+      jsx: true,
+    },
+  },
+});
+
+ruleTester.run(RULE_NAME, rule, {
+  valid: [
+    {
+      code: 'const atoms = snapshot.getLoadable(someState).getValue();',
+    },
+    {
+      code: 'const atoms = snapshot.getLoadable(someState(viewId)).getValue();',
+    },
+  ],
+  invalid: [
+    {
+      code: 'const atoms = await snapshot.getPromise(someState);',
+      errors: [
+        {
+          messageId: 'invalidWayToGetAtoms',
+        },
+      ],
+      output: 'const atoms = snapshot.getLoadable(someState).getValue();',
+    },
+    {
+      code: 'const atoms = await snapshot.getPromise(someState(viewId));',
+      errors: [
+        {
+          messageId: 'invalidWayToGetAtoms',
+        },
+      ],
+      output:
+        'const atoms = snapshot.getLoadable(someState(viewId)).getValue();',
+    },
+    {
+      code: 'const atoms = snapshot.getLoadable(someState).anotherMethod();',
+      errors: [
+        {
+          messageId: 'invalidWayToGetAtoms',
+        },
+      ],
+      output: 'const atoms = snapshot.getLoadable(someState).getValue();',
+    },
+  ],
+});

tools/eslint-rules/rules/use-getLoadable-and-getValue-to-get-atoms.ts

@@ -0,0 +1,88 @@
+import { ESLintUtils } from '@typescript-eslint/utils';
+
+// NOTE: The rule will be available in ESLint configs as "@nx/workspace-usage-getLoadable-and-getValue-to-get-atoms"
+export const RULE_NAME = 'use-getLoadable-and-getValue-to-get-atoms';
+
+export const rule = ESLintUtils.RuleCreator(() => __filename)({
+  name: RULE_NAME,
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Ensure you are using getLoadable and getValue',
+      recommended: 'recommended',
+    },
+    fixable: 'code',
+    schema: [],
+    messages: {
+      redundantAwait: 'Redundant await on non-promise',
+      invalidAccessorOnSnapshot:
+        "Expected to use method 'getLoadable()' on 'snapshot' but instead found '{{ propertyName }}'",
+      invalidWayToGetAtoms:
+        "Expected to use method 'getValue()' with 'getLoadable()' but instead found '{{ propertyName }}'",
+    },
+  },
+  defaultOptions: [],
+  create: (context) => ({
+    AwaitExpression: (node) => {
+      const { argument, range }: any = node;
+      if (
+        (argument.callee?.object?.callee?.object?.name === 'snapshot' &&
+          argument?.callee?.object?.callee?.property?.name === 'getLoadable') ||
+        (argument.callee?.object?.name === 'snapshot' &&
+          argument?.callee?.property?.name === 'getLoadable')
+      ) {
+        // remove await
+        context.report({
+          node,
+          messageId: 'redundantAwait',
+          data: {
+            propertyName: argument.callee.property.name,
+          },
+          fix: (fixer) => fixer.removeRange([range[0], range[0] + 5]),
+        });
+      }
+    },
+    MemberExpression: (node) => {
+      const { object, property }: any = node;
+
+      if (
+        object.callee?.type === 'MemberExpression' &&
+        object.callee.object?.name === 'snapshot' &&
+        object.callee.property?.name === 'getLoadable'
+      ) {
+        const propertyName = property.name;
+
+        if (propertyName !== 'getValue') {
+          context.report({
+            node: property,
+            messageId: 'invalidWayToGetAtoms',
+            data: {
+              propertyName,
+            },
+            // replace the property with `getValue`
+            fix: (fixer) => fixer.replaceText(property, 'getValue'),
+          });
+        }
+      }
+    },
+    CallExpression: (node) => {
+      const { callee }: any = node;
+
+      if (
+        callee.type === 'MemberExpression' &&
+        callee.object?.name === 'snapshot' &&
+        callee.property?.name === 'getPromise'
+      ) {
+        context.report({
+          node: callee.property,
+          messageId: 'invalidAccessorOnSnapshot',
+          data: {
+            propertyName: callee.property.name,
+          },
+          // Replace `getPromise` with `getLoadable`
+          fix: (fixer) => fixer.replaceText(callee.property, 'getLoadable'),
+        });
+      }
+    },
+  }),
+});