Remove the {...props} pattern and props coupling, and create an eslint rule for that (#1733)

* Remove the {...props} pattern and props coupling, and create an eslint rule for that

Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: Thiago Nascimbeni <tnascimbeni@gmail.com>

* Add another test to the new rule

Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: Thiago Nascimbeni <tnascimbeni@gmail.com>

---------

Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Co-authored-by: Thiago Nascimbeni <tnascimbeni@gmail.com>

docs/docs/developer/frontend/style-guide.mdx

@@ -85,6 +85,32 @@ const EmailField = ({ value }: OwnProps) => (
 );
 ```
 
+#### No Single Variable Prop Spreading in JSX Elements
+
+We discourage the use of single variable prop spreading in JSX elements, e.g., `{...props}`. This practice often leads to less readable and maintainable code as it's unclear what props are being passed down to the component.
+
+```tsx
+/* ❌ - Bad, spreads a single variable prop into the underlying component
+ */
+const MyComponent = (props: OwnProps) => {
+  return <OtherComponent {...props} />;
+}
+```
+
+```tsx
+/* ✅ - Good, Explicitly lists all props
+ *    - Enhances readability and maintainability
+ */ 
+const MyComponent = ({ prop1, prop2, prop3 }: OwnProps) => {
+  return <OtherComponent {...{ prop1, prop2, prop3 }} />;
+};
+```
+
+Rationale:
+- It's clearer to see at a glance which props are being passed down, making the code easier to understand and maintain.
+- It helps to prevent tight coupling between components via their props.
+- It's easier to catch misspelled or unused props with linting tools when props are listed explicitly
+
 ## JavaScript 
 
 ### Use nullish-coalescing operator `??`

front/.eslintrc.js

@@ -62,6 +62,7 @@ module.exports = {
     'simple-import-sort/exports': 'error',
     'twenty/effect-components': 'error',
     'twenty/no-hardcoded-colors': 'error',
+    'twenty/no-spread-props': 'error',
     'twenty/matching-state-variable': 'error',
     'twenty/sort-css-properties-alphabetically': 'error',
     'twenty/styled-components-prefixed-with-styled': 'error',

front/src/modules/activities/table/components/CellCommentChip.tsx

@@ -10,6 +10,7 @@ export const CellCommentChip = (props: CommentChipProps) => {
 
   return (
     <StyledCellWrapper>
+      {/* eslint-disable-next-line twenty/no-spread-props */}
       <CommentChip {...props} />
     </StyledCellWrapper>
   );

front/src/modules/auth/components/Logo.tsx

@@ -50,6 +50,7 @@ const StyledMainLogo = styled.div<StyledMainLogoProps>`
 export const Logo = ({ workspaceLogo, ...props }: Props) => {
   if (!workspaceLogo) {
     return (
+      // eslint-disable-next-line twenty/no-spread-props
       <StyledContainer {...props}>
         <StyledMainLogo logo="/icons/android/android-launchericon-192-192.png" />
       </StyledContainer>
@@ -57,6 +58,7 @@ export const Logo = ({ workspaceLogo, ...props }: Props) => {
   }
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledContainer {...props}>
       <StyledMainLogo logo={getImageAbsoluteURIOrBase64(workspaceLogo)} />
       <StyledTwentyLogoContainer>

front/src/modules/auth/components/Modal.tsx

@@ -11,6 +11,7 @@ const StyledContent = styled(UIModal.Content)`
 type Props = React.ComponentProps<'div'>;
 
 export const AuthModal = ({ children, ...restProps }: Props) => (
+  // eslint-disable-next-line twenty/no-spread-props
   <UIModal isOpen={true} {...restProps}>
     <StyledContent>{children}</StyledContent>
   </UIModal>

front/src/modules/auth/sign-in-up/components/FooterNote.tsx

@@ -11,4 +11,5 @@ const StyledContainer = styled.div`
   text-align: center;
 `;
 
+// eslint-disable-next-line twenty/no-spread-props
 export const FooterNote = (props: Props) => <StyledContainer {...props} />;

front/src/modules/spreadsheet-import/components/Heading.tsx

@@ -28,6 +28,7 @@ const StyledDescription = styled.span`
 `;
 
 export const Heading = ({ title, description, ...props }: Props) => (
+  // eslint-disable-next-line twenty/no-spread-props
   <StyledContainer {...props}>
     <StyledTitle>{title}</StyledTitle>
     {description && <StyledDescription>{description}</StyledDescription>}

front/src/modules/spreadsheet-import/components/Table.tsx

@@ -115,6 +115,7 @@ export const Table = <Data,>(props: Props<Data>) => {
   const { rtl } = useSpreadsheetImportInternal();
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledDataGrid direction={rtl ? 'rtl' : 'ltr'} rowHeight={52} {...props} />
   );
 };

front/src/modules/ui/button/components/MainButton.tsx

@@ -107,6 +107,7 @@ export const MainButton = ({
 }: MainButtonProps) => {
   const theme = useTheme();
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledButton fullWidth={fullWidth} variant={variant} {...props}>
       {Icon && <Icon size={theme.icon.size.sm} />}
       {title}

front/src/modules/ui/button/components/RoundedIconButton.tsx

@@ -40,6 +40,7 @@ export const RoundedIconButton = ({
   const theme = useTheme();
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledIconButton {...props}>
       {<Icon size={theme.icon.size.md} />}
     </StyledIconButton>

front/src/modules/ui/checkmark/components/AnimatedCheckmark.tsx

@@ -24,6 +24,7 @@ export const AnimatedCheckmark = ({
       height={size}
     >
       <motion.path
+        // eslint-disable-next-line twenty/no-spread-props
         {...restProps}
         fill="none"
         stroke={color ?? theme.grayScale.gray0}

front/src/modules/ui/checkmark/components/Checkmark.tsx

@@ -20,6 +20,7 @@ export const Checkmark = (props: CheckmarkProps) => {
   const theme = useTheme();
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledContainer {...props}>
       <IconCheck color={theme.grayScale.gray0} size={14} />
     </StyledContainer>

front/src/modules/ui/color-scheme/components/ColorSchemeCard.tsx

@@ -105,6 +105,7 @@ const ColorSchemeSegment = ({
   controls,
   ...rest
 }: ColorSchemeSegmentProps) => (
+  // eslint-disable-next-line twenty/no-spread-props
   <StyledColorSchemeBackground variant={variant} {...rest}>
     <StyledColorSchemeContent animate={controls} variant={variant}>
       Aa
@@ -173,6 +174,7 @@ export const ColorSchemeCard = ({
         <StyledMixedColorSchemeSegment
           onMouseEnter={handleMouseEnter}
           onMouseLeave={handleMouseLeave}
+          // eslint-disable-next-line twenty/no-spread-props
           {...rest}
         >
           <ColorSchemeSegment
@@ -211,6 +213,7 @@ export const ColorSchemeCard = ({
         onMouseLeave={handleMouseLeave}
         controls={controls}
         variant={variant}
+        // eslint-disable-next-line twenty/no-spread-props
         {...rest}
       />
       <AnimatePresence>

front/src/modules/ui/dialog/components/Dialog.tsx

@@ -137,6 +137,7 @@ export const Dialog = ({
       <StyledDialogContainer
         variants={containerVariants}
         transition={{ damping: 15, stiffness: 100 }}
+        // eslint-disable-next-line twenty/no-spread-props
         {...rootProps}
       >
         {title && <StyledDialogTitle>{title}</StyledDialogTitle>}
@@ -151,6 +152,7 @@ export const Dialog = ({
             }}
             fullWidth={true}
             variant={button.variant ?? 'secondary'}
+            // eslint-disable-next-line twenty/no-spread-props
             {...button}
           />
         ))}

front/src/modules/ui/dialog/components/DialogProvider.tsx

@@ -40,6 +40,7 @@ export const DialogProvider = ({ children }: React.PropsWithChildren) => {
       {dialogInternal.queue.map((dialog) => (
         <Dialog
           key={dialog.id}
+          // eslint-disable-next-line twenty/no-spread-props
           {...dialog}
           onClose={() => handleDialogClose(dialog.id)}
         />

front/src/modules/ui/dropdown/components/DropdownMenuContainer.tsx

@@ -39,6 +39,7 @@ export const DropdownMenuContainer = ({
   });
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledDropdownMenuContainer data-select-disable {...props} anchor={anchor}>
       <StyledDropdownMenu ref={dropdownRef} width={width}>
         {children}

front/src/modules/ui/dropdown/components/DropdownMenuHeader.tsx

@@ -55,6 +55,7 @@ export const DropdownMenuHeader = ({
   const theme = useTheme();
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledHeader {...props}>
       {StartIcon && (
         <StyledStartIconWrapper>

front/src/modules/ui/dropdown/components/DropdownMenuSearchInput.tsx

@@ -38,6 +38,7 @@ export const DropdownMenuSearchInput = forwardRef<
   InputHTMLAttributes<HTMLInputElement>
 >((props, ref) => (
   <StyledDropdownMenuSearchInputContainer>
+    {/* eslint-disable-next-line twenty/no-spread-props */}
     <StyledInput autoComplete="off" placeholder="Search" {...props} ref={ref} />
   </StyledDropdownMenuSearchInputContainer>
 ));

front/src/modules/ui/input/components/Radio.tsx

@@ -126,6 +126,7 @@ export const Radio = ({
   };
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledContainer {...restProps} labelPosition={labelPosition}>
       <StyledRadioInput
         type="radio"

front/src/modules/ui/input/image/components/ImageInput.tsx

@@ -104,6 +104,7 @@ export const ImageInput = ({
   };
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledContainer {...restProps}>
       <StyledPicture
         withPicture={!!picture}

front/src/modules/ui/input/relation-picker/components/SingleEntitySelect.tsx

@@ -71,6 +71,7 @@ export const SingleEntitySelect = <
       />
       <StyledDropdownMenuSeparator />
       <SingleEntitySelectBase
+        // eslint-disable-next-line twenty/no-spread-props
         {...props}
         onCancel={onCancel}
         onCreate={onCreate}

front/src/modules/ui/input/relation-picker/components/__stories__/SingleEntitySelect.stories.tsx

@@ -41,6 +41,7 @@ const meta: Meta<typeof SingleEntitySelect> = {
 
     return (
       <SingleEntitySelect
+        // eslint-disable-next-line twenty/no-spread-props
         {...args}
         entitiesToSelect={entities.filter(
           (entity) =>

front/src/modules/ui/input/text/components/TextInputSettings.tsx

@@ -170,6 +170,7 @@ const TextInputComponent = (
               onChange(event.target.value);
             }
           }}
+          // eslint-disable-next-line twenty/no-spread-props
           {...props}
         />
         <StyledTrailingIconContainer>

front/src/modules/ui/input/text/components/__stories__/TextInput.stories.tsx

@@ -28,6 +28,7 @@ const FakeTextInput = ({
   const [value, setValue] = useState(initialValue);
   return (
     <TextInputSettings
+      // eslint-disable-next-line twenty/no-spread-props
       {...props}
       value={value}
       onChange={(text) => {
@@ -41,6 +42,7 @@ const FakeTextInput = ({
 export const Default: Story = {
   argTypes: { value: { control: false } },
   args: { value: 'A good value ' },
+  // eslint-disable-next-line twenty/no-spread-props
   render: (args) => <FakeTextInput {...args} />,
   play: async ({ canvasElement }) => {
     const canvas = within(canvasElement);

front/src/modules/ui/layout/components/PageHeader.tsx

@@ -92,6 +92,7 @@ export const PageHeader = ({
   const theme = useTheme();
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledTopBarContainer {...props}>
       <StyledLeftContainer>
         {!isNavbarOpened && (

front/src/modules/ui/modal/components/Modal.tsx

@@ -99,18 +99,21 @@ const StyledBackDrop = styled(motion.div)`
 type ModalHeaderProps = React.PropsWithChildren & React.ComponentProps<'div'>;
 
 const ModalHeader = ({ children, ...restProps }: ModalHeaderProps) => (
+  // eslint-disable-next-line twenty/no-spread-props
   <StyledHeader {...restProps}>{children}</StyledHeader>
 );
 
 type ModalContentProps = React.PropsWithChildren & React.ComponentProps<'div'>;
 
 const ModalContent = ({ children, ...restProps }: ModalContentProps) => (
+  // eslint-disable-next-line twenty/no-spread-props
   <StyledContent {...restProps}>{children}</StyledContent>
 );
 
 type ModalFooterProps = React.PropsWithChildren & React.ComponentProps<'div'>;
 
 const ModalFooter = ({ children, ...restProps }: ModalFooterProps) => (
+  // eslint-disable-next-line twenty/no-spread-props
   <StyledFooter {...restProps}>{children}</StyledFooter>
 );
 
@@ -203,6 +206,7 @@ export const Modal = ({
         exit="exit"
         layout
         variants={modalVariants}
+        // eslint-disable-next-line twenty/no-spread-props
         {...restProps}
       >
         {children}

front/src/modules/ui/snack-bar/components/SnackBar.tsx

@@ -160,6 +160,7 @@ export const SnackBar = ({
       onMouseEnter={onMouseEnter}
       onMouseLeave={onMouseLeave}
       variant={variant}
+      // eslint-disable-next-line twenty/no-spread-props
       {...rootProps}
     >
       <StyledProgressBarContainer>

front/src/modules/ui/snack-bar/components/SnackBarProvider.tsx

@@ -80,6 +80,7 @@ export const SnackBarProvider = ({ children }: React.PropsWithChildren) => {
             layout
           >
             <SnackBar
+              // eslint-disable-next-line twenty/no-spread-props
               {...snackBar}
               onClose={() => handleSnackBarClose(snackBar.id)}
             />

front/src/modules/ui/step-bar/components/StepBar.tsx

@@ -25,6 +25,7 @@ export const StepBar = ({ children, activeStep, ...restProps }: StepsProps) => {
   const isMobile = useIsMobile();
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledContainer {...restProps}>
       {React.Children.map(children, (child, index) => {
         if (!React.isValidElement(child)) {

front/src/modules/ui/table/components/EntityTableColumnMenu.tsx

@@ -51,6 +51,7 @@ export const EntityTableColumnMenu = ({
   );
 
   return (
+    // eslint-disable-next-line twenty/no-spread-props
     <StyledColumnMenu {...props} ref={ref}>
       <StyledDropdownMenuItemsContainer>
         {hiddenTableColumns.map((column) => (

front/src/modules/ui/tooltip/AppTooltip.tsx

@@ -44,5 +44,6 @@ export type AppToolipProps = {
 };
 
 export const AppTooltip = (props: AppToolipProps) => (
+  // eslint-disable-next-line twenty/no-spread-props
   <StyledAppTooltip {...props} />
 );

front/src/modules/ui/tooltip/__stories__/Tooltip.stories.tsx

@@ -26,6 +26,7 @@ export const Default: Story = {
       <p id="hover-text" data-testid="tooltip">
         Hover me!
       </p>
+      {/* eslint-disable-next-line twenty/no-spread-props */}
       <Tooltip {...args} />
     </>
   ),

front/src/modules/ui/utilities/animation/components/AnimatedEaseIn.tsx

@@ -21,6 +21,7 @@ export const AnimatedEaseIn = ({
       initial={initial}
       animate={animate}
       transition={transition}
+      // eslint-disable-next-line twenty/no-spread-props
       {...restProps}
     >
       {children}

front/src/modules/ui/utilities/animation/components/AnimatedTextWord.tsx

@@ -58,6 +58,7 @@ export const AnimatedTextWord = ({ text = '', ...restProps }: Props) => {
       variants={containerAnimation}
       initial="hidden"
       animate="visible"
+      // eslint-disable-next-line twenty/no-spread-props
       {...restProps}
     >
       {words.map((word, index) => (

packages/eslint-plugin-twenty-ts/dist/index.js

@@ -4,6 +4,7 @@ module.exports = {
     rules: {
         "effect-components": require("./src/rules/effect-components"),
         "no-hardcoded-colors": require("./src/rules/no-hardcoded-colors"),
+        "no-spread-props": require('./src/rules/no-spread-props'),
         "matching-state-variable": require("./src/rules/matching-state-variable"),
         "sort-css-properties-alphabetically": require("./src/rules/sort-css-properties-alphabetically"),
         "styled-components-prefixed-with-styled": require("./src/rules/styled-components-prefixed-with-styled"),

packages/eslint-plugin-twenty-ts/dist/src/rules/no-spread-props.js

@@ -0,0 +1,31 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+const utils_1 = require("@typescript-eslint/utils");
+const createRule = utils_1.ESLintUtils.RuleCreator(() => "https://docs.twenty.com/developer/frontend/style-guide#no-single-variable-prop-spreading-in-jsx-elements");
+const noSpreadPropsRule = createRule({
+    create: (context) => ({
+        JSXSpreadAttribute: (node) => {
+            if (node.argument.type === "Identifier") {
+                context.report({
+                    node,
+                    messageId: "noSpreadProps",
+                });
+            }
+        },
+    }),
+    name: "no-spread-props",
+    meta: {
+        docs: {
+            description: "Disallow passing props as {...props} in React components.",
+        },
+        messages: {
+            noSpreadProps: `Single variable prop spreading is disallowed in JSX elements.\nPrefer explicitly listing out all props or using an object expression like so: \`{...{ prop1, prop2 }}\`.\nSee https://docs.twenty.com/developer/frontend/style-guide#no-single-variable-prop-spreading-in-jsx-elements for more information.`,
+        },
+        type: "suggestion",
+        schema: [],
+        fixable: "code",
+    },
+    defaultOptions: [],
+});
+module.exports = noSpreadPropsRule;
+exports.default = noSpreadPropsRule;

packages/eslint-plugin-twenty-ts/dist/src/tests/no-spread-props.spec.js

@@ -0,0 +1,34 @@
+"use strict";
+var __importDefault = (this && this.__importDefault) || function (mod) {
+    return (mod && mod.__esModule) ? mod : { "default": mod };
+};
+Object.defineProperty(exports, "__esModule", { value: true });
+const rule_tester_1 = require("@typescript-eslint/rule-tester");
+const no_spread_props_1 = __importDefault(require("../rules/no-spread-props"));
+const ruleTester = new rule_tester_1.RuleTester({
+    parser: "@typescript-eslint/parser",
+    parserOptions: {
+      project: "./tsconfig.json",
+      tsconfigRootDir: __dirname,
+      ecmaFeatures: {
+        jsx: true,
+      },
+    },
+  });
+ruleTester.run("no-spread-props", no_spread_props_1.default, {
+    valid: [
+        {
+          code: "<MyComponent prop1={value} prop2={value} />",
+        },
+      ],
+      invalid: [
+        {
+          code: "<MyComponent {...props} />",
+          errors: [
+            {
+              messageId: "noSpreadProps",
+            },
+          ],
+        },
+      ],
+});

packages/eslint-plugin-twenty/index.ts

@@ -2,6 +2,7 @@ module.exports = {
   rules: {
     "effect-components": require("./src/rules/effect-components"),
     "no-hardcoded-colors": require("./src/rules/no-hardcoded-colors"),
+    "no-spread-props": require("./src/rules/no-spread-props"),
     "matching-state-variable": require("./src/rules/matching-state-variable"),
     "sort-css-properties-alphabetically": require("./src/rules/sort-css-properties-alphabetically"),
     "styled-components-prefixed-with-styled": require("./src/rules/styled-components-prefixed-with-styled"),

packages/eslint-plugin-twenty/src/rules/no-spread-props.ts

@@ -0,0 +1,36 @@
+import { ESLintUtils } from "@typescript-eslint/utils";
+
+const createRule = ESLintUtils.RuleCreator(
+  () =>
+    "https://docs.twenty.com/developer/frontend/style-guide#no-single-variable-prop-spreading-in-jsx-elements",
+);
+
+const noSpreadPropsRule = createRule({
+  create: (context) => ({
+    JSXSpreadAttribute: (node) => {
+      if (node.argument.type === "Identifier") {
+        context.report({
+          node,
+          messageId: "noSpreadProps",
+        });
+      }
+    },
+  }),
+  name: "no-spread-props",
+  meta: {
+    docs: {
+      description: "Disallow passing props as {...props} in React components.",
+    },
+    messages: {
+      noSpreadProps: `Single variable prop spreading is disallowed in JSX elements.\nPrefer explicitly listing out all props or using an object expression like so: \`{...{ prop1, prop2 }}\`.\nSee https://docs.twenty.com/developer/frontend/style-guide#no-single-variable-prop-spreading-in-jsx-elements for more information.`,
+    },
+    type: "suggestion",
+    schema: [],
+    fixable: "code",
+  },
+  defaultOptions: [],
+});
+
+module.exports = noSpreadPropsRule;
+
+export default noSpreadPropsRule;

packages/eslint-plugin-twenty/src/tests/no-spread-props.spec.ts

@@ -0,0 +1,35 @@
+import { RuleTester } from "@typescript-eslint/rule-tester";
+
+import noSpreadPropsRule from "../rules/no-spread-props";
+
+const ruleTester = new RuleTester({
+  parser: "@typescript-eslint/parser",
+  parserOptions: {
+    project: "./tsconfig.json",
+    tsconfigRootDir: __dirname,
+    ecmaFeatures: {
+      jsx: true,
+    },
+  },
+});
+
+ruleTester.run("no-spread-props", noSpreadPropsRule, {
+  valid: [
+    {
+      code: "<MyComponent prop1={value} prop2={value} />",
+    },
+    {
+      code: "<MyComponent {...{prop1, prop2}} />",
+    },
+  ],
+  invalid: [
+    {
+      code: "<MyComponent {...props} />",
+      errors: [
+        {
+          messageId: "noSpreadProps",
+        },
+      ],
+    },
+  ],
+});