[No QA] Wrap personal details list access behind hooks, store and update builder

config/eslint/eslint.config.mjs

@@ -304,6 +304,7 @@ const config = defineConfig([
             'rulesdir/require-a11y-disable-justification': 'error',
             'rulesdir/no-direct-pre-insert-fullscreen-under-rhp': 'error',
             'rulesdir/no-raw-typography': 'error',
+            'rulesdir/no-direct-personal-details-list': 'error',
             'rulesdir/require-locale-for-localized-date-format': 'error',
             'rulesdir/prefer-narrow-hook-dependencies': [
                 'error',
@@ -652,6 +653,28 @@ const config = defineConfig([
         rules: {'report-name-utils/no-function-call-in-get-report-name': 'error'},
     },
 
+    // Everything else must read personal details through `@hooks/usePersonalDetails`, `@libs/PersonalDetailsStore` or
+    // `buildPersonalDetailsUpdate`, so that changing the shape of the personal details data means changing those
+    // wrappers instead of ~200 call sites. The files below are exempt because they are the wrappers themselves, the
+    // place the key is declared, or test setup that has to seed Onyx by key.
+    {
+        files: [
+            'src/ONYXKEYS.ts',
+            'src/hooks/usePersonalDetails.ts',
+            'src/libs/PersonalDetailsStore.ts',
+            'src/libs/PersonalDetailsUtils.ts',
+            'src/components/OnyxListItemProvider.tsx',
+            'src/libs/ExportOnyxState/common.ts',
+            'tests/**/*.{ts,tsx}',
+            'jest/**/*.{ts,tsx}',
+            '__mocks__/**/*.{ts,tsx}',
+            'src/**/__mocks__/**/*.{ts,tsx}',
+        ],
+        rules: {
+            'rulesdir/no-direct-personal-details-list': 'off',
+        },
+    },
+
     // The typography token files are where raw font sizes and line heights are defined.
     {
         files: ['src/styles/typography.ts', 'src/styles/variables.ts'],

cspell.json

@@ -920,6 +920,7 @@
         "QAPR",
         "QBWC",
         "qrcode",
+        "quasis",
         "quotepath",
         "rach",
         "reactnative",

eslint-plugin-local-rules/no-direct-personal-details-list.js

@@ -0,0 +1,61 @@
+const name = 'no-direct-personal-details-list';
+
+const meta = {
+    type: 'problem',
+    docs: {
+        description:
+            'Disallow direct use of `ONYXKEYS.PERSONAL_DETAILS_LIST`. Personal details are being reshaped into an Onyx collection, so access has to go through a wrapper that can absorb the change.',
+        recommended: 'error',
+    },
+    schema: [],
+    messages: {
+        directUsage:
+            '`ONYXKEYS.PERSONAL_DETAILS_LIST` is migrating to an Onyx collection, so do not reference it directly. In React use @hooks/usePersonalDetails, outside React use @libs/PersonalDetailsStore, and for writes use `buildPersonalDetailsUpdate` from @libs/PersonalDetailsUtils.',
+    },
+};
+
+const ONYXKEYS_IDENTIFIER = 'ONYXKEYS';
+const PERSONAL_DETAILS_LIST_PROPERTY = 'PERSONAL_DETAILS_LIST';
+
+/**
+ * Resolves the property name of a member expression when it is statically known, so `ONYXKEYS.PERSONAL_DETAILS_LIST`,
+ * `ONYXKEYS['PERSONAL_DETAILS_LIST']` and ``ONYXKEYS[`PERSONAL_DETAILS_LIST`]`` are all treated as the same access.
+ * A computed access through a variable stays unresolved, because its value is only known at runtime.
+ *
+ * @param {import('estree').MemberExpression} node
+ * @returns {string | undefined}
+ */
+function getStaticPropertyName(node) {
+    if (!node.computed) {
+        return node.property.type === 'Identifier' ? node.property.name : undefined;
+    }
+    if (node.property.type === 'Literal') {
+        return typeof node.property.value === 'string' ? node.property.value : undefined;
+    }
+    if (node.property.type === 'TemplateLiteral' && node.property.expressions.length === 0 && node.property.quasis.length === 1) {
+        return node.property.quasis[0].value.cooked;
+    }
+    return undefined;
+}
+
+/**
+ * @param {import('eslint').Rule.RuleContext} context
+ * @returns {import('eslint').Rule.RuleListener}
+ */
+function create(context) {
+    return {
+        // Type positions parse as TSTypeQuery, not MemberExpression, so `typeof ONYXKEYS.PERSONAL_DETAILS_LIST`
+        // is allowed: it neither reads nor writes, and it follows the key definition when that is reshaped.
+        MemberExpression(node) {
+            if (node.object.type !== 'Identifier' || node.object.name !== ONYXKEYS_IDENTIFIER) {
+                return;
+            }
+            if (getStaticPropertyName(node) !== PERSONAL_DETAILS_LIST_PROPERTY) {
+                return;
+            }
+            context.report({node, messageId: 'directUsage'});
+        },
+    };
+}
+
+export {name, meta, create};

knip.json

@@ -1,4 +1,9 @@
 {
+    "ignoreIssues": {
+        "src/hooks/usePersonalDetails.ts": ["files"],
+        "src/libs/PersonalDetailsStore.ts": ["exports"],
+        "src/libs/PersonalDetailsUtils.ts": ["exports"]
+    },
     "workspaces": {
         ".": {
             "entry": [

src/hooks/usePersonalDetails.ts

@@ -0,0 +1,25 @@
+/** Personal details reads for components, so nothing touches PERSONAL_DETAILS_LIST directly */
+import ONYXKEYS from '@src/ONYXKEYS';
+import {personalDetailsListSelector, personalDetailsSelector} from '@src/selectors/PersonalDetails';
+import type {PersonalDetails, PersonalDetailsList} from '@src/types/onyx';
+
+import type {UseOnyxResult} from 'react-native-onyx';
+
+import useOnyx from './useOnyx';
+
+function usePersonalDetail(accountID: number | undefined): UseOnyxResult<PersonalDetails | undefined> {
+    return useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: personalDetailsSelector(accountID)});
+}
+
+function usePersonalDetailsByIDs(accountIDs: Array<number | undefined> | undefined): UseOnyxResult<PersonalDetailsList> {
+    return useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: personalDetailsListSelector(accountIDs)});
+}
+
+// Re-renders on any account change, prefer usePersonalDetail/usePersonalDetailsByIDs when the accounts are known
+function useAllPersonalDetails(): UseOnyxResult<PersonalDetailsList | undefined>;
+function useAllPersonalDetails<TReturn>(selector: (value: PersonalDetailsList | undefined) => TReturn): UseOnyxResult<TReturn>;
+function useAllPersonalDetails<TReturn>(selector?: (value: PersonalDetailsList | undefined) => TReturn) {
+    return useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector});
+}
+
+export {usePersonalDetail, usePersonalDetailsByIDs, useAllPersonalDetails};

src/libs/PersonalDetailsStore.ts

@@ -0,0 +1,49 @@
+/** Personal details for non-React callers, and the one place the coming Onyx-collection reshape has to land */
+import ONYXKEYS from '@src/ONYXKEYS';
+import type {PersonalDetails, PersonalDetailsList} from '@src/types/onyx';
+
+import Onyx from 'react-native-onyx';
+
+let allPersonalDetails: PersonalDetailsList = {};
+let loginToPersonalDetails: Record<string, PersonalDetails> = {};
+
+function buildLoginMapping(personalDetailsList: PersonalDetailsList): Record<string, PersonalDetails> {
+    const mapping: Record<string, PersonalDetails> = {};
+    for (const personalDetail of Object.values(personalDetailsList)) {
+        if (!personalDetail?.login) {
+            continue;
+        }
+        const login = personalDetail.login.toLowerCase();
+        const existing = mapping[login];
+        if (!existing || existing.isClosed || existing.isOptimisticPersonalDetail) {
+            mapping[login] = personalDetail;
+        }
+    }
+    return mapping;
+}
+
+// Only sanctioned connectWithoutView for personal details, don't add another: React uses @hooks/usePersonalDetails, everything else uses this store
+Onyx.connectWithoutView({
+    key: ONYXKEYS.PERSONAL_DETAILS_LIST,
+    callback: (value) => {
+        allPersonalDetails = value ?? {};
+        loginToPersonalDetails = buildLoginMapping(allPersonalDetails);
+    },
+});
+
+function getAllPersonalDetails(): PersonalDetailsList {
+    return allPersonalDetails;
+}
+
+function getPersonalDetail(accountID: number | undefined): PersonalDetails | undefined {
+    return accountID ? (allPersonalDetails[accountID] ?? undefined) : undefined;
+}
+
+function getPersonalDetailByLogin(login: string | undefined): PersonalDetails | undefined {
+    if (!login) {
+        return undefined;
+    }
+    return loginToPersonalDetails[login.toLowerCase()];
+}
+
+export {getAllPersonalDetails, getPersonalDetail, getPersonalDetailByLogin};

src/libs/PersonalDetailsUtils.ts

@@ -9,7 +9,7 @@ import type {Address} from '@src/types/onyx/PrivatePersonalDetails';
 import type {OnyxData} from '@src/types/onyx/Request';
 import {isEmptyObject} from '@src/types/utils/EmptyObject';
 
-import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx';
+import type {OnyxEntry, OnyxMergeInput, OnyxUpdate} from 'react-native-onyx';
 import type {SetNonNullable} from 'type-fest';
 
 import {Str} from 'expensify-common';
@@ -18,6 +18,7 @@ import Onyx from 'react-native-onyx';
 import {getCountryCode} from './CountryUtils';
 import {translateLocal} from './Localize';
 import {areEmailsFromSamePrivateDomain} from './LoginUtils';
+import {getPersonalDetailByLogin} from './PersonalDetailsStore';
 import {addSMSDomainIfPhoneNumber, parsePhoneNumber} from './PhoneNumber';
 import {getDefaultAvatarURL} from './UserAvatarUtils';
 import {generateAccountID} from './UserUtils';
@@ -27,24 +28,6 @@ type FirstAndLastName = {
     lastName: string;
 };
 
-let emailToPersonalDetailsCache: Record<string, PersonalDetails> = {};
-Onyx.connect({
-    key: ONYXKEYS.PERSONAL_DETAILS_LIST,
-    callback: (val) => {
-        const personalDetails = Object.values(val ?? {});
-        emailToPersonalDetailsCache = personalDetails.reduce((acc: Record<string, PersonalDetails>, detail) => {
-            if (detail?.login) {
-                const key = detail.login.toLowerCase();
-                const existing = acc[key];
-                if (!existing || existing.isClosed || existing.isOptimisticPersonalDetail) {
-                    acc[key] = detail;
-                }
-            }
-            return acc;
-        }, {});
-    },
-});
-
 let hiddenTranslation = '';
 let youTranslation = '';
 
@@ -264,10 +247,7 @@ function getDisplayNameOrYou(displayName: string, accountID: number, currentUser
 }
 
 function getPersonalDetailByEmail(email: string | undefined): PersonalDetails | undefined {
-    if (!email) {
-        return undefined;
-    }
-    return emailToPersonalDetailsCache[email.toLowerCase()];
+    return getPersonalDetailByLogin(email);
 }
 
 /**
@@ -337,6 +317,17 @@ function getNewAccountIDsAndLogins(invitedEmailsToAccountIDs: InvitedEmailsToAcc
     );
 }
 
+type PersonalDetailsUpdate = OnyxMergeInput<typeof ONYXKEYS.PERSONAL_DETAILS_LIST>;
+
+// All writes are merges, and the builder is what turns one list update into one update per account after the reshape
+function buildPersonalDetailsUpdate(personalDetails: PersonalDetailsUpdate): OnyxUpdate<typeof ONYXKEYS.PERSONAL_DETAILS_LIST> {
+    return {
+        onyxMethod: Onyx.METHOD.MERGE,
+        key: ONYXKEYS.PERSONAL_DETAILS_LIST,
+        value: personalDetails,
+    };
+}
+
 /**
  * Given a list of logins and accountIDs, return Onyx data for users with no existing personal details stored. These users might be brand new or unknown.
  * They will have an "optimistic" accountID that must be cleaned up later.
@@ -637,4 +628,5 @@ export {
     areAddressAndPersonalDetailsMissing,
     areTravelPersonalDetailsMissing,
     temporaryGetDisplayNameOrDefault,
+    buildPersonalDetailsUpdate,
 };

tests/unit/NoDirectPersonalDetailsListRuleTest.ts

@@ -0,0 +1,96 @@
+import type {Rule} from 'eslint';
+
+import {RuleTester} from 'eslint';
+import {parser as tsParser} from 'typescript-eslint';
+
+type LocalRuleModule = Rule.RuleModule & {
+    name: string;
+};
+
+function isLocalRuleModule(ruleModule: unknown): ruleModule is LocalRuleModule {
+    if (typeof ruleModule !== 'object' || ruleModule === null) {
+        return false;
+    }
+
+    const ruleName: unknown = Reflect.get(ruleModule, 'name');
+    const create: unknown = Reflect.get(ruleModule, 'create');
+    const meta: unknown = Reflect.get(ruleModule, 'meta');
+
+    return typeof ruleName === 'string' && typeof create === 'function' && typeof meta === 'object' && meta !== null;
+}
+
+const ruleModule: unknown = require('../../eslint-plugin-local-rules/no-direct-personal-details-list');
+
+if (!isLocalRuleModule(ruleModule)) {
+    throw new TypeError('Expected no-direct-personal-details-list to export an ESLint rule module.');
+}
+
+const tsRuleTester = new RuleTester({
+    languageOptions: {
+        ecmaVersion: 2022,
+        sourceType: 'module',
+        parser: tsParser,
+        parserOptions: {
+            ecmaFeatures: {
+                jsx: true,
+            },
+        },
+    },
+});
+
+describe('no-direct-personal-details-list', () => {
+    tsRuleTester.run(ruleModule.name, ruleModule, {
+        valid: [
+            'const [personalDetail] = usePersonalDetail(accountID);',
+            'const [personalDetails] = usePersonalDetailsByIDs(accountIDs);',
+            'const personalDetail = getPersonalDetail(accountID);',
+            // Type positions parse as TSTypeQuery, not MemberExpression.
+            'type Update = OnyxUpdate<typeof ONYXKEYS.PERSONAL_DETAILS_LIST>;',
+            'type Data = Partial<Record<typeof ONYXKEYS.PERSONAL_DETAILS_LIST, PersonalDetailsList>>;',
+            'const [metadata] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_METADATA);',
+            'const [list] = useOnyx(ONYXKEYS.COLLECTION.REPORT);',
+            'const key = SOME_OTHER_KEYS.PERSONAL_DETAILS_LIST;',
+            // Only known at runtime, so there is nothing to resolve statically.
+            'const key = ONYXKEYS[PERSONAL_DETAILS_LIST];',
+        ],
+        invalid: [
+            {
+                code: 'const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);',
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                code: 'const [login] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: personalDetailsLoginSelector(accountID)});',
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                code: "const [personalDetails] = useOnyx(ONYXKEYS['PERSONAL_DETAILS_LIST']);",
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                code: 'const [personalDetails] = useOnyx(ONYXKEYS[`PERSONAL_DETAILS_LIST`]);',
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                code: 'Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[accountID]: null});',
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                code: 'const update = {onyxMethod: Onyx.METHOD.MERGE, key: ONYXKEYS.PERSONAL_DETAILS_LIST, value: personalDetails};',
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                code: 'Onyx.connectWithoutView({key: ONYXKEYS.PERSONAL_DETAILS_LIST, callback: (value) => value});',
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                code: 'const masks = {[ONYXKEYS.PERSONAL_DETAILS_LIST]: {allowList: []}};',
+                errors: [{messageId: 'directUsage'}],
+            },
+            {
+                // Reported per usage, so the seatbelt count is per usage too.
+                code: 'const [all] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {});',
+                errors: [{messageId: 'directUsage'}, {messageId: 'directUsage'}],
+            },
+        ],
+    });
+});