Feat/performance-refactor-styled-component (twentyhq#5516)

In this PR I'm optimizing a whole RecordTableCell in real conditions with a complex RelationFieldDisplay component : - Broke down getObjectRecordIdentifier into multiple utils - Precompute memoized function for getting chip data per field with useRecordChipDataGenerator() - Refactored RelationFieldDisplay - Use CSS modules where performance is needed instead of styled components - Create a CSS theme with global CSS variables to be used by CSS modules
synapsenet-arena · May 24, 2024 · a017847 · a017847
1 parent 3680647
commit a017847
Show file tree

Hide file tree

Showing 39 changed files with 1,044 additions and 461 deletions.
diff --git a/package.json b/package.json
@@ -72,7 +72,7 @@
     "bullmq": "^4.14.0",
     "bytes": "^3.1.2",
     "class-transformer": "^0.5.1",
-    "clsx": "^1.2.1",
+    "clsx": "^2.1.1",
     "cross-env": "^7.0.3",
     "crypto-js": "^4.2.0",
     "danger-plugin-todos": "^1.3.1",

diff --git a/packages/twenty-front/.storybook/preview.tsx b/packages/twenty-front/.storybook/preview.tsx
@@ -1,3 +1,4 @@
+import { useEffect } from 'react';
 import { ThemeProvider } from '@emotion/react';
 import { Preview } from '@storybook/react';
 import { initialize, mswDecorator } from 'msw-storybook-addon';
@@ -31,6 +32,11 @@ const preview: Preview = {
     (Story) => {
       const theme = useDarkMode() ? THEME_DARK : THEME_LIGHT;
 
+      useEffect(() => {
+        document.documentElement.className =
+          theme.name === 'dark' ? 'dark' : 'light';
+      }, [theme]);
+
       return (
         <ThemeProvider theme={theme}>
           <Story />

diff --git a/packages/twenty-front/.storybook/test-runner-jest.config.js b/packages/twenty-front/.storybook/test-runner-jest.config.js
@@ -11,5 +11,5 @@ export default {
   /** Add your own overrides below
    * @see https://jestjs.io/docs/configuration
    */
-  testTimeout: 2 * MINUTES_IN_MS,
+  testTimeout: 5 * MINUTES_IN_MS,
 };
diff --git a/packages/twenty-front/src/modules/object-metadata/utils/getAvatarType.ts b/packages/twenty-front/src/modules/object-metadata/utils/getAvatarType.ts
@@ -0,0 +1,13 @@
+import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
+
+export const getAvatarType = (objectNameSingular: string) => {
+  if (objectNameSingular === CoreObjectNameSingular.WorkspaceMember) {
+    return 'rounded';
+  }
+
+  if (objectNameSingular === CoreObjectNameSingular.Company) {
+    return 'squared';
+  }
+
+  return 'rounded';
+};
diff --git a/packages/twenty-front/src/modules/object-metadata/utils/getAvatarUrl.ts b/packages/twenty-front/src/modules/object-metadata/utils/getAvatarUrl.ts
@@ -0,0 +1,36 @@
+import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
+import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
+import { ObjectRecord } from '@/object-record/types/ObjectRecord';
+import { getLogoUrlFromDomainName } from '~/utils';
+import { isDefined } from '~/utils/isDefined';
+
+import { getImageIdentifierFieldValue } from './getImageIdentifierFieldValue';
+
+export const getAvatarUrl = (
+  objectNameSingular: string,
+  record: ObjectRecord,
+  imageIdentifierFieldMetadataItem: FieldMetadataItem | undefined,
+) => {
+  if (objectNameSingular === CoreObjectNameSingular.WorkspaceMember) {
+    return record.avatarUrl ?? undefined;
+  }
+
+  if (objectNameSingular === CoreObjectNameSingular.Company) {
+    return getLogoUrlFromDomainName(record.domainName ?? '');
+  }
+
+  if (objectNameSingular === CoreObjectNameSingular.Person) {
+    return record.avatarUrl ?? '';
+  }
+
+  const imageIdentifierFieldValue = getImageIdentifierFieldValue(
+    record,
+    imageIdentifierFieldMetadataItem,
+  );
+
+  if (isDefined(imageIdentifierFieldValue)) {
+    return imageIdentifierFieldValue;
+  }
+
+  return '';
+};
diff --git a/packages/twenty-front/src/modules/object-metadata/utils/getImageIdentifierFieldValue.ts b/packages/twenty-front/src/modules/object-metadata/utils/getImageIdentifierFieldValue.ts
@@ -0,0 +1,14 @@
+import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
+import { ObjectRecord } from '@/object-record/types/ObjectRecord';
+import { isDefined } from '~/utils/isDefined';
+
+export const getImageIdentifierFieldValue = (
+  record: ObjectRecord,
+  imageIdentifierFieldMetadataItem: FieldMetadataItem | undefined,
+) => {
+  if (isDefined(imageIdentifierFieldMetadataItem?.name)) {
+    return record[imageIdentifierFieldMetadataItem.name] as string;
+  }
+
+  return null;
+};
diff --git a/packages/twenty-front/src/modules/object-metadata/utils/getLabelIdentifierFieldValue.ts b/packages/twenty-front/src/modules/object-metadata/utils/getLabelIdentifierFieldValue.ts
@@ -0,0 +1,24 @@
+import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
+import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
+import { ObjectRecord } from '@/object-record/types/ObjectRecord';
+import { FieldMetadataType } from '~/generated-metadata/graphql';
+import { isDefined } from '~/utils/isDefined';
+
+export const getLabelIdentifierFieldValue = (
+  record: ObjectRecord,
+  labelIdentifierFieldMetadataItem: FieldMetadataItem | undefined,
+  objectNameSingular: string,
+) => {
+  if (
+    objectNameSingular === CoreObjectNameSingular.WorkspaceMember ||
+    labelIdentifierFieldMetadataItem?.type === FieldMetadataType.FullName
+  ) {
+    return `${record.name?.firstName ?? ''} ${record.name?.lastName ?? ''}`;
+  }
+
+  if (isDefined(labelIdentifierFieldMetadataItem?.name)) {
+    return record[labelIdentifierFieldMetadataItem.name] as string | number;
+  }
+
+  return '';
+};
diff --git a/packages/twenty-front/src/modules/object-metadata/utils/getLinkToShowPage.ts b/packages/twenty-front/src/modules/object-metadata/utils/getLinkToShowPage.ts
@@ -0,0 +1,22 @@
+import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
+import { getBasePathToShowPage } from '@/object-metadata/utils/getBasePathToShowPage';
+import { ObjectRecord } from '@/object-record/types/ObjectRecord';
+
+export const getLinkToShowPage = (
+  objectNameSingular: string,
+  record: ObjectRecord,
+) => {
+  const basePathToShowPage = getBasePathToShowPage({
+    objectNameSingular,
+  });
+
+  const isWorkspaceMemberObjectMetadata =
+    objectNameSingular === CoreObjectNameSingular.WorkspaceMember;
+
+  const linkToShowPage =
+    isWorkspaceMemberObjectMetadata || !record.id
+      ? ''
+      : `${basePathToShowPage}${record.id}`;
+
+  return linkToShowPage;
+};
diff --git a/packages/twenty-front/src/modules/object-metadata/utils/getObjectRecordIdentifier.ts b/packages/twenty-front/src/modules/object-metadata/utils/getObjectRecordIdentifier.ts
@@ -1,12 +1,12 @@
-import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
 import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
-import { getBasePathToShowPage } from '@/object-metadata/utils/getBasePathToShowPage';
 import { getLabelIdentifierFieldMetadataItem } from '@/object-metadata/utils/getLabelIdentifierFieldMetadataItem';
 import { ObjectRecord } from '@/object-record/types/ObjectRecord';
 import { ObjectRecordIdentifier } from '@/object-record/types/ObjectRecordIdentifier';
-import { WorkspaceMember } from '@/workspace-member/types/WorkspaceMember';
-import { FieldMetadataType } from '~/generated-metadata/graphql';
-import { getLogoUrlFromDomainName } from '~/utils';
+
+import { getAvatarType } from './getAvatarType';
+import { getAvatarUrl } from './getAvatarUrl';
+import { getLabelIdentifierFieldValue } from './getLabelIdentifierFieldValue';
+import { getLinkToShowPage } from './getLinkToShowPage';
 
 export const getObjectRecordIdentifier = ({
   objectMetadataItem,
@@ -15,69 +15,32 @@ export const getObjectRecordIdentifier = ({
   objectMetadataItem: ObjectMetadataItem;
   record: ObjectRecord;
 }): ObjectRecordIdentifier => {
-  switch (objectMetadataItem.nameSingular) {
-    case CoreObjectNameSingular.WorkspaceMember: {
-      const workspaceMember = record as Partial<WorkspaceMember> & {
-        id: string;
-      };
-
-      const name = workspaceMember.name
-        ? `${workspaceMember.name?.firstName ?? ''} ${
-            workspaceMember.name?.lastName ?? ''
-          }`
-        : '';
-
-      return {
-        id: workspaceMember.id,
-        name,
-        avatarUrl: workspaceMember.avatarUrl ?? undefined,
-        avatarType: 'rounded',
-      };
-    }
-  }
-
   const labelIdentifierFieldMetadataItem =
     getLabelIdentifierFieldMetadataItem(objectMetadataItem);
 
-  const labelIdentifierFieldValue =
-    labelIdentifierFieldMetadataItem?.type === FieldMetadataType.FullName
-      ? `${record.name?.firstName ?? ''} ${record.name?.lastName ?? ''}`
-      : labelIdentifierFieldMetadataItem?.name
-        ? (record[labelIdentifierFieldMetadataItem.name] as string | number)
-        : '';
+  const labelIdentifierFieldValue = getLabelIdentifierFieldValue(
+    record,
+    labelIdentifierFieldMetadataItem,
+    objectMetadataItem.nameSingular,
+  );
 
   const imageIdentifierFieldMetadata = objectMetadataItem.fields.find(
     (field) => field.id === objectMetadataItem.imageIdentifierFieldMetadataId,
   );
 
-  const imageIdentifierFieldValue = imageIdentifierFieldMetadata
-    ? (record[imageIdentifierFieldMetadata.name] as string)
-    : null;
-
-  const avatarType =
-    objectMetadataItem.nameSingular === CoreObjectNameSingular.Company
-      ? 'squared'
-      : 'rounded';
+  const avatarType = getAvatarType(objectMetadataItem.nameSingular);
 
   // TODO: This is a temporary solution before we seed imageIdentifierFieldMetadataId in the database
-  const avatarUrl =
-    (objectMetadataItem.nameSingular === CoreObjectNameSingular.Company
-      ? getLogoUrlFromDomainName(record.domainName ?? '')
-      : objectMetadataItem.nameSingular === CoreObjectNameSingular.Person
-        ? record.avatarUrl ?? ''
-        : imageIdentifierFieldValue) ?? '';
-
-  const basePathToShowPage = getBasePathToShowPage({
-    objectNameSingular: objectMetadataItem.nameSingular,
-  });
-
-  const isWorkspaceMemberObjectMetadata =
-    objectMetadataItem.nameSingular === CoreObjectNameSingular.WorkspaceMember;
+  const avatarUrl = getAvatarUrl(
+    objectMetadataItem.nameSingular,
+    record,
+    imageIdentifierFieldMetadata,
+  );
 
-  const linkToShowPage =
-    isWorkspaceMemberObjectMetadata || !record.id
-      ? ''
-      : `${basePathToShowPage}${record.id}`;
+  const linkToShowPage = getLinkToShowPage(
+    objectMetadataItem.nameSingular,
+    record,
+  );
 
   return {
     id: record.id,

diff --git a/packages/twenty-front/src/modules/object-record/components/RecordChip.tsx b/packages/twenty-front/src/modules/object-record/components/RecordChip.tsx
@@ -7,15 +7,13 @@ import { getImageAbsoluteURIOrBase64 } from '~/utils/image/getImageAbsoluteURIOr
 export type RecordChipProps = {
   objectNameSingular: string;
   record: ObjectRecord;
-  maxWidth?: number;
   className?: string;
   variant?: EntityChipVariant;
 };
 
 export const RecordChip = ({
   objectNameSingular,
   record,
-  maxWidth,
   className,
   variant,
 }: RecordChipProps) => {
@@ -34,7 +32,6 @@ export const RecordChip = ({
         getImageAbsoluteURIOrBase64(objectRecordIdentifier.avatarUrl) || ''
       }
       linkToEntity={objectRecordIdentifier.linkToShowPage}
-      maxWidth={maxWidth}
       className={className}
       variant={variant}
     />

diff --git a/...modules/object-record/record-field/meta-types/display/components/RelationFieldDisplay.tsx b/...modules/object-record/record-field/meta-types/display/components/RelationFieldDisplay.tsx
@@ -1,9 +1,12 @@
-import { RecordChip } from '@/object-record/components/RecordChip';
+import { EntityChip } from 'twenty-ui';
+
 import { useRelationFieldDisplay } from '@/object-record/record-field/meta-types/hooks/useRelationFieldDisplay';
-import { ObjectRecord } from '@/object-record/types/ObjectRecord';
+import { getImageAbsoluteURIOrBase64 } from '~/utils/image/getImageAbsoluteURIOrBase64';
+import { isDefined } from '~/utils/isDefined';
 
 export const RelationFieldDisplay = () => {
-  const { fieldValue, fieldDefinition, maxWidth } = useRelationFieldDisplay();
+  const { fieldValue, fieldDefinition, generateRecordChipData } =
+    useRelationFieldDisplay();
 
   if (
     !fieldValue ||
@@ -12,13 +15,21 @@ export const RelationFieldDisplay = () => {
     return null;
   }
 
+  if (!isDefined(generateRecordChipData)) {
+    throw new Error(
+      `generateRecordChipData is not defined for field ${fieldDefinition.metadata.fieldName}, this should not happen. Check your RecordTableContext to see if it's correctly initialized.`,
+    );
+  }
+
+  const recordChipData = generateRecordChipData(fieldValue);
+
   return (
-    <RecordChip
-      objectNameSingular={
-        fieldDefinition.metadata.relationObjectMetadataNameSingular
-      }
-      record={fieldValue as unknown as ObjectRecord} // Todo: Fix this type
-      maxWidth={maxWidth}
+    <EntityChip
+      entityId={fieldValue.id}
+      name={recordChipData.name as any}
+      avatarType={recordChipData.avatarType}
+      avatarUrl={getImageAbsoluteURIOrBase64(recordChipData.avatarUrl) || ''}
+      linkToEntity={recordChipData.linkToShowPage}
     />
   );
 };
diff --git a/...ield/meta-types/display/components/__stories__/perf/RelationFieldDisplay.perf.stories.tsx b/...ield/meta-types/display/components/__stories__/perf/RelationFieldDisplay.perf.stories.tsx
@@ -10,8 +10,11 @@ import {
   useSetRecordValue,
 } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';
 import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
+import { RecordTableContext } from '@/object-record/record-table/contexts/RecordTableContext';
+import { ObjectRecord } from '@/object-record/types/ObjectRecord';
 import { MemoryRouterDecorator } from '~/testing/decorators/MemoryRouterDecorator';
 import { getProfilingStory } from '~/testing/profiling/utils/getProfilingStory';
+import { getLogoUrlFromDomainName } from '~/utils';
 
 import { relationFieldDisplayMock } from './mock';
 
@@ -49,20 +52,35 @@ const meta: Meta = {
     MemoryRouterDecorator,
     (Story) => (
       <RecordFieldValueSelectorContextProvider>
-        <FieldContext.Provider
-          value={{
-            entityId: relationFieldDisplayMock.entityId,
-            basePathToShowPage: '/object-record/',
-            isLabelIdentifier: false,
-            fieldDefinition: {
-              ...relationFieldDisplayMock.fieldDefinition,
-            },
-            hotkeyScope: 'hotkey-scope',
-          }}
+        <RecordTableContext.Provider
+          value={
+            {
+              recordChipDataGeneratorPerFieldName: {
+                company: (objectRecord: ObjectRecord) => ({
+                  name: objectRecord.name,
+                  avatarType: 'rounded',
+                  avatarUrl: getLogoUrlFromDomainName(objectRecord.domainName),
+                  linkToShowPage: '/object-record/company',
+                }),
+              },
+            } as any
+          }
         >
-          <RelationFieldValueSetterEffect />
-          <Story />
-        </FieldContext.Provider>
+          <FieldContext.Provider
+            value={{
+              entityId: relationFieldDisplayMock.entityId,
+              basePathToShowPage: '/object-record/',
+              isLabelIdentifier: false,
+              fieldDefinition: {
+                ...relationFieldDisplayMock.fieldDefinition,
+              },
+              hotkeyScope: 'hotkey-scope',
+            }}
+          >
+            <RelationFieldValueSetterEffect />
+            <Story />
+          </FieldContext.Provider>
+        </RecordTableContext.Provider>
       </RecordFieldValueSelectorContextProvider>
     ),
     ComponentDecorator,