From e5e3132ddda14b7d97808210432e2c32a81e28a5 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Thu, 5 Mar 2026 17:58:52 +0100 Subject: [PATCH] Add ESLint rules to disallow jotaiStore and direct atomFamily usage in selectors (#18422) Introduce two new ESLint rules that prevent the use of `jotaiStore` and direct calls to `.atomFamily()` or `.selectorFamily()` within component selector `get` callbacks. These rules promote cleaner and more reactive code practices. Fixed file touched by those new rules : `calendarDayRecordIdsComponentFamilySelector` --- .../eslint.config.react.mjs | 2 + packages/twenty-eslint-rules/index.ts | 10 ++ .../no-direct-atom-family-in-selector.spec.ts | 90 +++++++++++++++++ .../no-direct-atom-family-in-selector.ts | 78 +++++++++++++++ .../rules/no-jotai-store-in-selector.spec.ts | 99 +++++++++++++++++++ .../rules/no-jotai-store-in-selector.ts | 88 +++++++++++++++++ .../utils/isNodeInsideAncestor.ts | 18 ++++ ...lendarDayRecordsComponentFamilySelector.ts | 22 ++--- 8 files changed, 392 insertions(+), 15 deletions(-) create mode 100644 packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.spec.ts create mode 100644 packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.ts create mode 100644 packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.spec.ts create mode 100644 packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.ts create mode 100644 packages/twenty-eslint-rules/utils/isNodeInsideAncestor.ts diff --git a/packages/twenty-eslint-rules/eslint.config.react.mjs b/packages/twenty-eslint-rules/eslint.config.react.mjs index b960d1e966..a805e1e793 100644 --- a/packages/twenty-eslint-rules/eslint.config.react.mjs +++ b/packages/twenty-eslint-rules/eslint.config.react.mjs @@ -566,6 +566,8 @@ export default [ 'twenty/explicit-boolean-predicates-in-if': 'error', 'twenty/no-navigate-prefer-link': 'error', + 'twenty/no-jotai-store-in-selector': 'error', + 'twenty/no-direct-atom-family-in-selector': 'error', }, }, diff --git a/packages/twenty-eslint-rules/index.ts b/packages/twenty-eslint-rules/index.ts index d899090979..e6adf09927 100644 --- a/packages/twenty-eslint-rules/index.ts +++ b/packages/twenty-eslint-rules/index.ts @@ -34,10 +34,18 @@ import { rule as noAngleBracketPlaceholders, RULE_NAME as noAngleBracketPlaceholdersName, } from './rules/no-angle-bracket-placeholders'; +import { + rule as noDirectAtomFamilyInSelector, + RULE_NAME as noDirectAtomFamilyInSelectorName, +} from './rules/no-direct-atom-family-in-selector'; import { rule as noHardcodedColors, RULE_NAME as noHardcodedColorsName, } from './rules/no-hardcoded-colors'; +import { + rule as noJotaiStoreInSelector, + RULE_NAME as noJotaiStoreInSelectorName, +} from './rules/no-jotai-store-in-selector'; import { rule as noNavigatePreferLink, RULE_NAME as noNavigatePreferLinkName, @@ -98,6 +106,8 @@ module.exports = { [explicitBooleanPredicatesInIfName]: explicitBooleanPredicatesInIf, [maxConstsPerFileName]: maxConstsPerFile, [noNavigatePreferLinkName]: noNavigatePreferLink, + [noJotaiStoreInSelectorName]: noJotaiStoreInSelector, + [noDirectAtomFamilyInSelectorName]: noDirectAtomFamilyInSelector, [injectWorkspaceRepositoryName]: injectWorkspaceRepository, [restApiMethodsShouldBeGuardedName]: restApiMethodsShouldBeGuarded, [graphqlResolversShouldBeGuardedName]: graphqlResolversShouldBeGuarded, diff --git a/packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.spec.ts b/packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.spec.ts new file mode 100644 index 0000000000..d9c3e842e9 --- /dev/null +++ b/packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.spec.ts @@ -0,0 +1,90 @@ +import { TSESLint } from '@typescript-eslint/utils'; + +import { rule, RULE_NAME } from './no-direct-atom-family-in-selector'; + +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + }, +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + const mySelector = createAtomComponentSelector({ + key: 'mySelector', + get: ({ instanceId }) => ({ get }) => { + const value = get(someComponentState, { instanceId }); + return value; + }, + componentInstanceContext: MyContext, + }); + `, + }, + { + code: ` + const mySelector = createAtomComponentFamilySelector({ + key: 'mySelector', + get: ({ instanceId, familyKey }) => ({ get }) => { + const record = get(recordStoreFamilyState, familyKey.recordId); + return record; + }, + componentInstanceContext: MyContext, + }); + `, + }, + { + code: ` + const atom = someState.atomFamily({ instanceId: 'test' }); + `, + }, + ], + invalid: [ + { + code: ` + const mySelector = createAtomComponentSelector({ + key: 'mySelector', + get: ({ instanceId }) => ({ get }) => { + const atom = someState.atomFamily({ instanceId }); + return get(atom); + }, + componentInstanceContext: MyContext, + }); + `, + errors: [{ messageId: 'noDirectAtomFamilyInSelector' }], + }, + { + code: ` + const mySelector = createAtomComponentFamilySelector({ + key: 'mySelector', + get: ({ instanceId, familyKey }) => ({ get }) => { + const selector = someSelector.selectorFamily({ instanceId }); + return get(selector); + }, + componentInstanceContext: MyContext, + }); + `, + errors: [{ messageId: 'noDirectAtomFamilyInSelector' }], + }, + { + code: ` + const mySelector = createAtomComponentFamilySelector({ + key: 'mySelector', + get: ({ instanceId, familyKey }) => ({ get }) => { + const a = someState.atomFamily({ instanceId }); + const b = otherState.selectorFamily({ instanceId }); + return get(a) + get(b); + }, + componentInstanceContext: MyContext, + }); + `, + errors: [ + { messageId: 'noDirectAtomFamilyInSelector' }, + { messageId: 'noDirectAtomFamilyInSelector' }, + ], + }, + ], +}); diff --git a/packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.ts b/packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.ts new file mode 100644 index 0000000000..8a338238d6 --- /dev/null +++ b/packages/twenty-eslint-rules/rules/no-direct-atom-family-in-selector.ts @@ -0,0 +1,78 @@ +import { + AST_NODE_TYPES, + ESLintUtils, + type TSESTree, +} from '@typescript-eslint/utils'; + +import { isNodeInsideAncestor } from '../utils/isNodeInsideAncestor'; + +export const RULE_NAME = 'no-direct-atom-family-in-selector'; + +const SELECTOR_FACTORY_NAMES = [ + 'createAtomComponentSelector', + 'createAtomComponentFamilySelector', +]; + +export const rule = ESLintUtils.RuleCreator(() => __filename)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Disallow direct .atomFamily() or .selectorFamily() calls inside component selector get callbacks', + }, + messages: { + noDirectAtomFamilyInSelector: + 'Do not call `.atomFamily()` or `.selectorFamily()` directly inside component selector `get` callbacks. Use the provided `get` helper instead, because it is the cleanest API.', + }, + schema: [], + }, + defaultOptions: [], + create: (context) => { + const selectorGetNodes: TSESTree.Node[] = []; + + return { + CallExpression: (node) => { + if ( + node.callee.type !== AST_NODE_TYPES.Identifier || + !SELECTOR_FACTORY_NAMES.includes(node.callee.name) + ) { + return; + } + + const configArg = node.arguments[0]; + + if ( + !configArg || + configArg.type !== AST_NODE_TYPES.ObjectExpression + ) { + return; + } + + const getProperty = configArg.properties.find( + (prop): prop is TSESTree.Property => + prop.type === AST_NODE_TYPES.Property && + prop.key.type === AST_NODE_TYPES.Identifier && + prop.key.name === 'get', + ); + + if (getProperty) { + selectorGetNodes.push(getProperty.value); + } + }, + + 'CallExpression > MemberExpression[property.name=/^(atomFamily|selectorFamily)$/]': + (node: TSESTree.MemberExpression) => { + for (const getNode of selectorGetNodes) { + if (isNodeInsideAncestor(node, getNode)) { + context.report({ + node, + messageId: 'noDirectAtomFamilyInSelector', + }); + break; + } + } + }, + }; + }, +}); diff --git a/packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.spec.ts b/packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.spec.ts new file mode 100644 index 0000000000..6892a3ce4e --- /dev/null +++ b/packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.spec.ts @@ -0,0 +1,99 @@ +import { TSESLint } from '@typescript-eslint/utils'; + +import { rule, RULE_NAME } from './no-jotai-store-in-selector'; + +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + const mySelector = createAtomComponentSelector({ + key: 'mySelector', + get: ({ instanceId }) => ({ get }) => { + const value = get(someComponentState, { instanceId }); + return value; + }, + componentInstanceContext: MyContext, + }); + `, + }, + { + code: ` + const mySelector = createAtomComponentFamilySelector({ + key: 'mySelector', + get: ({ instanceId, familyKey }) => ({ get }) => { + const messages = get(messagesComponentState, { instanceId }); + return messages.find((m) => m.id === familyKey.messageId); + }, + componentInstanceContext: MyContext, + }); + `, + }, + { + code: ` + const value = jotaiStore.get(someState.atom); + `, + }, + ], + invalid: [ + { + code: ` + const mySelector = createAtomComponentSelector({ + key: 'mySelector', + get: ({ instanceId }) => ({ get }) => { + const value = jotaiStore.get(someState.atom); + return value; + }, + componentInstanceContext: MyContext, + }); + `, + errors: [{ messageId: 'noJotaiStoreInSelector' }], + }, + { + code: ` + const mySelector = createAtomComponentFamilySelector({ + key: 'mySelector', + get: ({ instanceId, familyKey }) => ({ get }) => { + const value = jotaiStore.get(someState.atom); + return value; + }, + componentInstanceContext: MyContext, + }); + `, + errors: [{ messageId: 'noJotaiStoreInSelector' }], + }, + { + code: ` + const mySelector = createAtomComponentSelector({ + key: 'mySelector', + get: ({ instanceId }) => ({ get }) => { + jotaiStore.set(someState.atom, 'value'); + return 'value'; + }, + componentInstanceContext: MyContext, + }); + `, + errors: [{ messageId: 'noJotaiStoreInSelector' }], + }, + { + code: ` + const mySelector = createAtomComponentFamilySelector({ + key: 'mySelector', + get: ({ instanceId, familyKey }) => ({ get }) => { + const a = jotaiStore.get(stateA.atom); + const b = jotaiStore.get(stateB.atom); + return a + b; + }, + componentInstanceContext: MyContext, + }); + `, + errors: [ + { messageId: 'noJotaiStoreInSelector' }, + { messageId: 'noJotaiStoreInSelector' }, + ], + }, + ], +}); diff --git a/packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.ts b/packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.ts new file mode 100644 index 0000000000..368b36e577 --- /dev/null +++ b/packages/twenty-eslint-rules/rules/no-jotai-store-in-selector.ts @@ -0,0 +1,88 @@ +import { + AST_NODE_TYPES, + ESLintUtils, + type TSESTree, +} from '@typescript-eslint/utils'; + +import { isNodeInsideAncestor } from '../utils/isNodeInsideAncestor'; + +export const RULE_NAME = 'no-jotai-store-in-selector'; + +const SELECTOR_FACTORY_NAMES = [ + 'createAtomComponentSelector', + 'createAtomComponentFamilySelector', +]; + +export const rule = ESLintUtils.RuleCreator(() => __filename)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Disallow using jotaiStore inside component selector get callbacks', + }, + messages: { + noJotaiStoreInSelector: + 'Do not use `jotaiStore` inside component selector `get` callbacks. Use the provided `get` helper from the callback argument instead, because it breaks the reactivity loop.', + }, + schema: [], + }, + defaultOptions: [], + create: (context) => { + const selectorGetNodes: TSESTree.Node[] = []; + + return { + CallExpression: (node) => { + if ( + node.callee.type !== AST_NODE_TYPES.Identifier || + !SELECTOR_FACTORY_NAMES.includes(node.callee.name) + ) { + return; + } + + const configArg = node.arguments[0]; + + if ( + !configArg || + configArg.type !== AST_NODE_TYPES.ObjectExpression + ) { + return; + } + + const getProperty = configArg.properties.find( + (prop): prop is TSESTree.Property => + prop.type === AST_NODE_TYPES.Property && + prop.key.type === AST_NODE_TYPES.Identifier && + prop.key.name === 'get', + ); + + if (getProperty) { + selectorGetNodes.push(getProperty.value); + } + }, + + 'MemberExpression > Identifier[name="jotaiStore"]': ( + node: TSESTree.Identifier, + ) => { + const memberExpr = node.parent; + + if ( + !memberExpr || + memberExpr.type !== AST_NODE_TYPES.MemberExpression + ) { + return; + } + + for (const getNode of selectorGetNodes) { + if (isNodeInsideAncestor(node, getNode)) { + context.report({ + node: memberExpr, + messageId: 'noJotaiStoreInSelector', + }); + break; + } + } + }, + }; + }, +}); diff --git a/packages/twenty-eslint-rules/utils/isNodeInsideAncestor.ts b/packages/twenty-eslint-rules/utils/isNodeInsideAncestor.ts new file mode 100644 index 0000000000..34893b3708 --- /dev/null +++ b/packages/twenty-eslint-rules/utils/isNodeInsideAncestor.ts @@ -0,0 +1,18 @@ +import { type TSESTree } from '@typescript-eslint/utils'; + +export const isNodeInsideAncestor = ( + node: TSESTree.Node, + ancestor: TSESTree.Node, +): boolean => { + let nextParent: TSESTree.Node | undefined = node.parent; + + while (nextParent) { + if (nextParent === ancestor) { + return true; + } + + nextParent = nextParent.parent; + } + + return false; +}; diff --git a/packages/twenty-front/src/modules/object-record/record-calendar/states/selectors/calendarDayRecordsComponentFamilySelector.ts b/packages/twenty-front/src/modules/object-record/record-calendar/states/selectors/calendarDayRecordsComponentFamilySelector.ts index 2267655eea..68470cf3bc 100644 --- a/packages/twenty-front/src/modules/object-record/record-calendar/states/selectors/calendarDayRecordsComponentFamilySelector.ts +++ b/packages/twenty-front/src/modules/object-record/record-calendar/states/selectors/calendarDayRecordsComponentFamilySelector.ts @@ -1,5 +1,4 @@ import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; -import { jotaiStore } from '@/ui/utilities/state/jotai/jotaiStore'; import { hasObjectMetadataItemPositionField } from '@/object-metadata/utils/hasObjectMetadataItemPositionField'; import { RecordCalendarComponentInstanceContext } from '@/object-record/record-calendar/states/contexts/RecordCalendarComponentInstanceContext'; @@ -23,13 +22,11 @@ export const calendarDayRecordIdsComponentFamilySelector = get: ({ instanceId, familyKey: { day, timeZone } }) => ({ get }) => { - const calendarFieldMetadataId = jotaiStore.get( - recordIndexCalendarFieldMetadataIdState.atom, + const calendarFieldMetadataId = get( + recordIndexCalendarFieldMetadataIdState, ); - const objectMetadataItems = jotaiStore.get( - objectMetadataItemsState.atom, - ); + const objectMetadataItems = get(objectMetadataItemsState); const objectMetadataItem = objectMetadataItems.find( (objectMetadataItem) => objectMetadataItem.fields.some( @@ -56,9 +53,8 @@ export const calendarDayRecordIdsComponentFamilySelector = }); const recordIds = allRecordIds.filter((recordId) => { - const record = jotaiStore.get( - recordStoreFamilyState.atomFamily(recordId), - ); + const record = get(recordStoreFamilyState, recordId); + const recordDate = record?.[fieldMetadataItem.name]; if (!isNonEmptyString(recordDate)) { @@ -80,12 +76,8 @@ export const calendarDayRecordIdsComponentFamilySelector = hasObjectMetadataItemPositionField(objectMetadataItem) ) { return recordIds.sort((a, b) => { - const recordA = jotaiStore.get( - recordStoreFamilyState.atomFamily(a), - ); - const recordB = jotaiStore.get( - recordStoreFamilyState.atomFamily(b), - ); + const recordA = get(recordStoreFamilyState, a); + const recordB = get(recordStoreFamilyState, b); const positionA = recordA?.position; const positionB = recordB?.position;