From ece7a384df155ac41826e17370d69db2a8e80b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Wed, 17 Jun 2026 13:40:55 +0200 Subject: [PATCH] fix(front): wait for viewFields + fieldMetadataItems before opening the metadata gate (#21713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem On twenty-main, loading a record-index/standalone page for the first time renders the page chrome (title, view chip with record count) but the table body stays blank. A subsequent reload fixes it. Regression introduced by the cache-first `currentUser` bootstrap (#21532); follow-up to #21592, which already mentioned the experiment "should be reviewed." ## Root cause (concurrency) The metadata loader runs in two phases and the gate opens between them: 1. **`loadMinimalMetadata`** fast-paths `objectMetadataItems` and `views` to `status: 'up-to-date'` with only their *minimal* fields. `viewFields` and `fieldMetadataItems` stay `'empty'`. 2. **`IsMinimalMetadataReadyEffect`** opens the gate as soon as those two are `'up-to-date'` — before viewFields exist. 3. The page mounts. `viewsSelector` joins views with an empty `viewFields` collection, so `view.viewFields = []`. `RecordIndexLoadBaseOnContextStoreEffect` calls `loadRecordIndexStates(view, …)` with the empty viewFields and pins `loadedViewId === contextStoreCurrentViewId`. 4. `loadStaleMetadataEntities` later populates viewFields; the selector recomputes, but the effect bails out on the `loadedViewId` guard. `currentRecordFields` stays empty. 5. `visibleRecordFields` stays empty → `RecordTableVirtualizedInitialDataLoadEffect` hits its `isEmpty(visibleRecordFields)` guard and never fetches → empty body. The "300" count visible in the screenshot comes from `useGetRecordIndexTotalCount`'s separate aggregate query, which doesn't depend on viewFields. **Why the gate close/reopen self-heal doesn't work reliably:** `replaceDraft → applyChanges` happen in the same microtask chain. React 18 automatic batching collapses both into a single render where status goes `'empty' → 'up-to-date'` without an intermediate `'draft-pending'` observable to React. The gate never closes, children never unmount, `loadedViewId` is never reset. **Why it surfaced after #21532:** Before, `currentUser` was loaded only after `GetCurrentUser` returned — by which time `loadStaleMetadataEntities` had typically also completed and viewFields were populated when the gate opened. Now the cached `currentUser` lets the gate open the moment `loadMinimalMetadata` finishes. ## Fix Extend `IsMinimalMetadataReadyEffect` to also require `fieldMetadataItems` and `viewFields` to be `'up-to-date'` before opening the gate. Both are joined into the data the record-index page reads on first paint (`objectMetadataItemsWithFieldsSelector` reads fieldMetadataItems; `viewsSelector` reads viewFields), so the page can't render correctly without them. - **Warm cache** (all entities hydrated `up-to-date` from IndexedDB): unaffected — gate opens immediately. - **Cold cache and the first load post–IndexedDB-migration**: the gate stays closed until `loadStaleMetadataEntities` + `applyChanges` finish, then opens with full metadata. The page mounts once with a populated view; no race. ## Tests - [x] `nx typecheck twenty-front` clean (file change passes `oxlint` on the touched file). - [ ] Manual on twenty-main: cold reload + first navigation to a record-index page renders the table body without needing a second reload. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01DD3469JAWYURa2sKUTJ85e --- _Generated by [Claude Code](https://claude.ai/code/session_01DD3469JAWYURa2sKUTJ85e)_ Review in cubic --------- Co-authored-by: Claude --- .../IsMinimalMetadataReadyEffect.tsx | 23 +++-- .../rules/matching-state-variable.spec.ts | 17 ++++ .../rules/matching-state-variable.ts | 83 ++++++++++++++----- 3 files changed, 99 insertions(+), 24 deletions(-) diff --git a/packages/twenty-front/src/modules/metadata-store/effect-components/IsMinimalMetadataReadyEffect.tsx b/packages/twenty-front/src/modules/metadata-store/effect-components/IsMinimalMetadataReadyEffect.tsx index 165c3c1413..35275a8bb1 100644 --- a/packages/twenty-front/src/modules/metadata-store/effect-components/IsMinimalMetadataReadyEffect.tsx +++ b/packages/twenty-front/src/modules/metadata-store/effect-components/IsMinimalMetadataReadyEffect.tsx @@ -14,15 +14,22 @@ export const IsMinimalMetadataReadyEffect = () => { const hasAccessTokenPair = useHasAccessTokenPair(); const currentUser = useAtomStateValue(currentUserState); const currentWorkspace = useAtomStateValue(currentWorkspaceState); - const metadataStore = useAtomFamilyStateValue( + const metadataStoreObjectMetadataItems = useAtomFamilyStateValue( metadataStoreState, 'objectMetadataItems', ); - // oxlint-disable-next-line twenty/matching-state-variable + const metadataStoreFieldMetadataItems = useAtomFamilyStateValue( + metadataStoreState, + 'fieldMetadataItems', + ); const metadataStoreViews = useAtomFamilyStateValue( metadataStoreState, 'views', ); + const metadataStoreViewFields = useAtomFamilyStateValue( + metadataStoreState, + 'viewFields', + ); const setIsMinimalMetadataReady = useSetAtomState( isMinimalMetadataReadyState, ); @@ -35,8 +42,12 @@ export const IsMinimalMetadataReadyEffect = () => { const hasActiveWorkspace = isWorkspaceActiveOrSuspended(currentWorkspace); - const areObjectsLoaded = metadataStore.status === 'up-to-date'; - const areViewsLoaded = metadataStoreViews.status === 'up-to-date'; + const areObjectsLoaded = + metadataStoreObjectMetadataItems.status === 'up-to-date' && + metadataStoreFieldMetadataItems.status === 'up-to-date'; + const areViewsLoaded = + metadataStoreViews.status === 'up-to-date' && + metadataStoreViewFields.status === 'up-to-date'; if (!areObjectsLoaded) { setIsMinimalMetadataReady(false); @@ -53,8 +64,10 @@ export const IsMinimalMetadataReadyEffect = () => { hasAccessTokenPair, currentUser, currentWorkspace, - metadataStore.status, + metadataStoreObjectMetadataItems.status, + metadataStoreFieldMetadataItems.status, metadataStoreViews.status, + metadataStoreViewFields.status, setIsMinimalMetadataReady, ]); diff --git a/packages/twenty-oxlint-rules/rules/matching-state-variable.spec.ts b/packages/twenty-oxlint-rules/rules/matching-state-variable.spec.ts index 95623bb550..4d7d1a173e 100644 --- a/packages/twenty-oxlint-rules/rules/matching-state-variable.spec.ts +++ b/packages/twenty-oxlint-rules/rules/matching-state-variable.spec.ts @@ -26,6 +26,18 @@ ruleTester.run(RULE_NAME, rule, { { code: 'const [variable, setVariable] = useAtomComponentFamilyState(variableComponentFamilyState, key);', }, + { + code: "const variableViews = useAtomFamilyStateValue(variableFamilyState, 'views');", + }, + { + code: "const variableViewFields = useAtomFamilyStateValue(variableFamilyState, 'viewFields');", + }, + { + code: "const [variableViews, setVariableViews] = useAtomComponentFamilyState(variableComponentFamilyState, 'views');", + }, + { + code: "const setVariableViews = useSetAtomFamilyState(variableFamilyState, 'views');", + }, { code: 'const setVariable = useSetAtomState(variableState);', }, @@ -114,5 +126,10 @@ ruleTester.run(RULE_NAME, rule, { output: 'const setVariable = useSetAtomComponentFamilyState(variableComponentFamilyState, key);', errors: [{ messageId: 'invalidSetterName' }], }, + { + code: "const variableSorts = useAtomFamilyStateValue(variableFamilyState, 'views');", + output: "const variable = useAtomFamilyStateValue(variableFamilyState, 'views');", + errors: [{ messageId: 'invalidVariableName' }], + }, ], }); diff --git a/packages/twenty-oxlint-rules/rules/matching-state-variable.ts b/packages/twenty-oxlint-rules/rules/matching-state-variable.ts index 0c7bb9cf0e..e7d7a094b8 100644 --- a/packages/twenty-oxlint-rules/rules/matching-state-variable.ts +++ b/packages/twenty-oxlint-rules/rules/matching-state-variable.ts @@ -22,6 +22,14 @@ const SETTER_HOOKS = [ 'useSetAtomComponentFamilyState', ]; +const FAMILY_HOOKS = new Set([ + 'useAtomFamilyStateValue', + 'useAtomComponentFamilyStateValue', + 'useAtomComponentFamilyState', + 'useSetAtomFamilyState', + 'useSetAtomComponentFamilyState', +]); + const ALL_HOOKS = [...VALUE_HOOKS, ...STATE_HOOKS, ...SETTER_HOOKS]; const SUFFIX_PATTERN = @@ -33,6 +41,31 @@ const getExpectedBaseName = (stateArgName: string): string => const getExpectedSetterName = (baseName: string): string => `set${baseName.charAt(0).toUpperCase()}${baseName.slice(1)}`; +// Family hooks may suffix the string-literal key to disambiguate multiple +// reads of the same family in one scope (e.g. metadataStore / metadataStoreViews). +const getFamilyKeyVariants = ( + hookName: string, + args: ReadonlyArray | undefined, +): string[] => { + if (!FAMILY_HOOKS.has(hookName) || !args) { + return []; + } + + const familyKeyArg = args[1]; + + if ( + familyKeyArg?.type !== 'Literal' || + typeof familyKeyArg.value !== 'string' || + familyKeyArg.value.length === 0 + ) { + return []; + } + + const keyValue = familyKeyArg.value; + + return [`${keyValue.charAt(0).toUpperCase()}${keyValue.slice(1)}`]; +}; + export const rule = defineRule({ meta: { type: 'problem', @@ -73,23 +106,37 @@ export const rule = defineRule({ const expectedVariableNameBase = getExpectedBaseName(stateNameBase); + const familyKeySuffixes = getFamilyKeyVariants( + hookName, + node.init.arguments, + ); + + const acceptedVariableNames = [ + expectedVariableNameBase, + ...familyKeySuffixes.map( + (suffix) => `${expectedVariableNameBase}${suffix}`, + ), + ]; + + const acceptedSetterNames = acceptedVariableNames.map( + getExpectedSetterName, + ); + if (SETTER_HOOKS.includes(hookName)) { if (node.id?.type === 'Identifier') { const actualName = node.id.name; - const expectedSetterName = - getExpectedSetterName(expectedVariableNameBase); - if (actualName !== expectedSetterName) { + if (!acceptedSetterNames.includes(actualName)) { context.report({ node, messageId: 'invalidSetterName', data: { hookName: stateNameBase, actualName, - expectedName: expectedSetterName, + expectedName: acceptedSetterNames[0], }, fix: (fixer) => - fixer.replaceText(node.id, expectedSetterName), + fixer.replaceText(node.id, acceptedSetterNames[0]), }); } } @@ -101,18 +148,18 @@ export const rule = defineRule({ if (node.id?.type === 'Identifier') { const actualName = node.id.name; - if (actualName !== expectedVariableNameBase) { + if (!acceptedVariableNames.includes(actualName)) { context.report({ node, messageId: 'invalidVariableName', data: { actualName, - expectedName: expectedVariableNameBase, + expectedName: acceptedVariableNames[0], hookName: stateNameBase, callee: hookName, }, fix: (fixer) => - fixer.replaceText(node.id, expectedVariableNameBase), + fixer.replaceText(node.id, acceptedVariableNames[0]), }); } } @@ -123,18 +170,18 @@ export const rule = defineRule({ if (node.id?.type === 'Identifier') { const actualVariableName = node.id.name; - if (actualVariableName !== expectedVariableNameBase) { + if (!acceptedVariableNames.includes(actualVariableName)) { context.report({ node, messageId: 'invalidVariableName', data: { actualName: actualVariableName, - expectedName: expectedVariableNameBase, + expectedName: acceptedVariableNames[0], hookName: stateNameBase, callee: hookName, }, fix: (fixer) => - fixer.replaceText(node.id, expectedVariableNameBase), + fixer.replaceText(node.id, acceptedVariableNames[0]), }); } @@ -149,14 +196,14 @@ export const rule = defineRule({ if ( actualVariableName && - actualVariableName !== expectedVariableNameBase + !acceptedVariableNames.includes(actualVariableName) ) { context.report({ node, messageId: 'invalidVariableName', data: { actualName: actualVariableName, - expectedName: expectedVariableNameBase, + expectedName: acceptedVariableNames[0], hookName: stateNameBase, callee: hookName, }, @@ -164,7 +211,7 @@ export const rule = defineRule({ if (node.id.type === 'ArrayPattern') { return fixer.replaceText( node.id.elements[0] as any, - expectedVariableNameBase, + acceptedVariableNames[0], ); } return null; @@ -174,23 +221,21 @@ export const rule = defineRule({ if (node.id.elements[1]?.type === 'Identifier') { const actualSetterName = node.id.elements[1].name; - const expectedSetterName = - getExpectedSetterName(expectedVariableNameBase); - if (actualSetterName !== expectedSetterName) { + if (!acceptedSetterNames.includes(actualSetterName)) { context.report({ node, messageId: 'invalidSetterName', data: { hookName: stateNameBase, actualName: actualSetterName, - expectedName: expectedSetterName, + expectedName: acceptedSetterNames[0], }, fix: (fixer) => { if (node.id.type === 'ArrayPattern') { return fixer.replaceText( node.id.elements[1]!, - expectedSetterName, + acceptedSetterNames[0], ); } return null;