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

[lucasbordeau]

Scope & Context

When working in useRecoilCallback, we have to get atoms that corresponds to the current state of the recoil store.

There are two ways to get an atom's value from the store : synchronous (with getLoadable) and asynchronous (with getPromise).

We want to enforce the usage of synchronous get, because in practice, we don't use async selectors or getters in our recoil atoms.

The problem with mixing asynchronous and synchronous getters, without any specific pattern, is that some getters will get executed before others and some getters won't get executed at all.

With synchronous getters, everything stays deterministic, as we really don't need more complexity in our state management layer.

We want to avoid this :

const savedFilters = await snapshot
  .getPromise(savedFiltersFamilyState(viewId));

And enforce this :

const savedFilters = snapshot
  .getLoadable(savedFiltersFamilyState(viewId))
  .getValue();

The existing .getPromise() have already been replaced in the codebase.

[Kanav-Arora]

Hi @lucasbordeau Can I work on this issue?

[lucasbordeau]

Hi @Kanav-Arora, sure please take on this issue, please join the Discord if you haven't already so we can discuss the details : https://discord.gg/bNgBwJny

[Kanav-Arora]

Hi @lucasbordeau Is there a directory for keeping custom eslint rules?

[lucasbordeau]

Sure you'll find plenty of examples in packages/eslint-plugin-twenty/src/rules

[FelixMalfait]

@Kanav-Arora should we unsassign you from this?

Here are suggestions from ChatGPT:

import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';

export const RULE_NAME = 'no-getpromise';

export const rule = ESLintUtils.RuleCreator(name => name)({
  name: RULE_NAME,
  meta: {
    type: 'problem',
    docs: {
      description: 'Enforce use of .getLoadable().getValue() instead of .getPromise()',
    },
    messages: {
      noGetPromise: 'Use .getLoadable().getValue() instead of .getPromise()',
    },
    schema: [],
    fixable: 'code',
  },
  defaultOptions: [],
  create: context => {
    return {
      MemberExpression(node: TSESTree.MemberExpression) {
        if (node.property.type === 'Identifier' && node.property.name === 'getPromise') {
          context.report({
            node,
            messageId: 'noGetPromise',
          });
        }
      },
    };
  },
});

And the spec file:

import { TSESLint } from '@typescript-eslint/experimental-utils';
import { rule, RULE_NAME } from './no-getpromise';

const ruleTester = new TSESLint.RuleTester({
  parser: require.resolve('@typescript-eslint/parser'),
});

ruleTester.run(RULE_NAME, rule, {
  valid: [
    {
      code: 'const savedFilters = snapshot.getLoadable(savedFiltersFamilyState(viewId)).getValue();',
    },
  ],
  invalid: [
    {
      code: 'const savedFilters = snapshot.getPromise(savedFiltersFamilyState(viewId));',
      errors: [
        {
          messageId: 'noGetPromise',
        },
      ],
    },
  ],
});

Warning: Not tested / this could be completely wrong

[Kanav-Arora]

Hi I tried this but I'll give it a try again

[FelixMalfait]

Cool thanks (again and again!) @Kanav-Arora

[gitstart-app[bot]]

Here is the GitStart Ticket for this issue: https://clients.gitstart.com/twenty/5449/tickets/TWNTY-2244