fix(front): wait for viewFields + fieldMetadataItems before opening the metadata gate (#21713)
## 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)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21713?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Félix Malfait
+18
-5
@@ -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,
|
||||
]);
|
||||
|
||||
|
||||
@@ -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' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
@@ -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<any> | 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;
|
||||
|
||||
Reference in New Issue
Block a user