chore(packages) avoid data-lgid and data-testid attributes being duplicated in components

.changeset/young-candies-appear.md

@@ -0,0 +1,14 @@
+---
+'@leafygreen-ui/confirmation-modal': patch
+'@leafygreen-ui/password-input': patch
+'@leafygreen-ui/progress-bar': patch
+'@leafygreen-ui/form-field': patch
+'@lg-tools/eslint-plugin': patch
+'@leafygreen-ui/checkbox': patch
+'@leafygreen-ui/select': patch
+'@leafygreen-ui/modal': patch
+'@leafygreen-ui/lib': patch
+'@lg-tools/lint': patch
+---
+
+Avoid duplicated data-lgid and data-testid

packages/checkbox/src/Checkbox/Checkbox.tsx

@@ -137,8 +137,8 @@ const Checkbox = React.forwardRef(
             className={cx(labelStyle, labelHoverStyle[theme], {
               [disabledLabelStyle]: disabled,
             })}
-            data-lgid={lgIds.root}
-            data-testid={lgIds.root}
+            data-lgid={lgIds.label}
+            data-testid={lgIds.label}
           >
             <input
               {...rest}
@@ -186,8 +186,8 @@ const Checkbox = React.forwardRef(
             <Description
               className={descriptionStyle}
               disabled={disabled}
-              data-lgid={lgIds.root}
-              data-testid={lgIds.root}
+              data-lgid={lgIds.description}
+              data-testid={lgIds.description}
             >
               {description}
             </Description>

packages/checkbox/src/utils/getLgIds.ts

@@ -5,6 +5,8 @@ export const DEFAULT_LGID_ROOT = 'lg-checkbox';
 export const getLgIds = (root: LgIdString = DEFAULT_LGID_ROOT) => {
   const ids = {
     root,
+    label: `${root}-label`,
+    description: `${root}-description`,
   } as const;
   return ids;
 };

packages/form-field/src/FormField/FormField.tsx

@@ -94,8 +94,8 @@ export const FormField = forwardRef<HTMLDivElement, FormFieldProps>(
             >
               {label && (
                 <Label
-                  data-lgid={lgIds.root}
-                  data-testid={lgIds.root}
+                  data-lgid={lgIds.label}
+                  data-testid={lgIds.label}
                   className={fontStyles}
                   htmlFor={inputId}
                   id={labelId}
@@ -106,8 +106,8 @@ export const FormField = forwardRef<HTMLDivElement, FormFieldProps>(
               )}
               {description && (
                 <Description
-                  data-lgid={lgIds.root}
-                  data-testid={lgIds.root}
+                  data-lgid={lgIds.description}
+                  data-testid={lgIds.description}
                   className={fontStyles}
                   id={descriptionId}
                   disabled={disabled}

packages/password-input/src/PasswordInput/PasswordInput.tsx

@@ -115,7 +115,7 @@ export const PasswordInput = React.forwardRef<
           className={className}
           ref={forwardedRef}
           data-lgid={lgIds.root}
-          data-test={lgIds.root}
+          data-testid={lgIds.root}
         >
           {label && (
             <Label
@@ -124,8 +124,8 @@ export const PasswordInput = React.forwardRef<
               })}
               htmlFor={inputId}
               disabled={disabled}
-              data-lgid={lgIds.root}
-              data-testid={lgIds.root}
+              data-lgid={lgIds.label}
+              data-testid={lgIds.label}
             >
               {label}
             </Label>

packages/password-input/src/utils/getLgIds.ts

@@ -6,6 +6,7 @@ export const getLgIds = (root: LgIdString = DEFAULT_LGID_ROOT) => {
   const ids = {
     root,
     stateNotifications: `${root}-state_notifications`,
+    label: `${root}-label`,
   } as const;
   return ids;
 };

packages/progress-bar/src/ProgressBar/ProgressBar.tsx

@@ -145,7 +145,7 @@ export function ProgressBar(props: ProgressBarProps) {
       >
         <div className={headerStyles}>
           <Label
-            data-lgid={lgIds.root} // handled by <Label> internally
+            data-lgid={lgIds.label} // handled by <Label> internally
             id={labelId}
             htmlFor={barId}
             className={truncatedTextStyles}
@@ -209,7 +209,7 @@ export function ProgressBar(props: ProgressBarProps) {
           <Description
             id={descId}
             disabled={disabled}
-            data-lgid={lgIds.root} // handled by <Description> internally
+            data-lgid={lgIds.description} // handled by <Description> internally
             className={cx({ [getAnimatedTextStyles()]: isNewDescription })}
             // if on fade-in transition, reset state after animation ends
             onAnimationEnd={() => setIsNewDescription(false)}

packages/progress-bar/src/testing/getTestUtils.ts

@@ -13,6 +13,8 @@ export const getLgIds = (root: `lg-${string}` = DEFAULT_LGID_ROOT) => {
     track: `${root}-track`,
     fill: `${root}-fill`,
     valueText: `${root}-value_text`,
+    label: `${root}-label`,
+    description: `${root}-description`,
   } as const;
 };
 

packages/select/src/Select.stories.tsx

@@ -143,7 +143,7 @@ export const LiveExample: StoryFn<SelectProps> = ({
 }: SelectProps) => (
   <Select
     {...args}
-    data-test="hello-world"
+    data-testid="hello-world"
     className={cx(
       css`
         min-width: 200px;

packages/select/src/Select/Select.tsx

@@ -542,8 +542,8 @@ export const Select = forwardRef<HTMLDivElement, SelectProps>(
             <div className={labelDescriptionContainerStyle}>
               {label && (
                 <Label
-                  data-lgid={lgIds.root}
-                  data-testid={lgIds.root}
+                  data-lgid={lgIds.label}
+                  data-testid={lgIds.label}
                   htmlFor={menuButtonId}
                   id={labelId}
                   darkMode={darkMode}
@@ -574,8 +574,8 @@ export const Select = forwardRef<HTMLDivElement, SelectProps>(
 
               {description && (
                 <Description
-                  data-lgid={lgIds.root}
-                  data-testid={lgIds.root}
+                  data-lgid={lgIds.description}
+                  data-testid={lgIds.description}
                   id={descriptionId}
                   darkMode={darkMode}
                   disabled={disabled}

packages/select/src/utils/getLgIds.ts

@@ -9,6 +9,8 @@ export const getLgIds = (root: LgIdString = DEFAULT_LGID_ROOT) => {
     popover: `${root}-popover`,
     trigger: `${root}-trigger`,
     buttonText: `${root}-button_text`,
+    label: `${root}-label`,
+    description: `${root}-description`,
   } as const;
   return ids;
 };

packages/typography/src/Description/Description.spec.tsx

@@ -1,18 +1,10 @@
-import React, { ReactNode } from 'react';
+import React from 'react';
 import { render } from '@testing-library/react';
 
-import { PolymorphicAs } from '@leafygreen-ui/polymorphic';
-
 import Description from './Description';
 
-const renderDescription = ({
-  as,
-  children = 'Test description',
-}: {
-  as?: PolymorphicAs;
-  children?: ReactNode;
-}) => {
-  return render(<Description as={as}>{children}</Description>);
+const renderDescription = (props: React.ComponentProps<typeof Description>) => {
+  return render(<Description {...props} />);
 };
 
 describe('Description Component', () => {
@@ -37,4 +29,23 @@ describe('Description Component', () => {
     });
     expect(container.querySelector('div')).toBeInTheDocument();
   });
+
+  test('renders with a default data-lgid', () => {
+    const { container } = renderDescription({ children: 'Test description' });
+    expect(container.querySelector('p')).toHaveAttribute(
+      'data-lgid',
+      'lg-typography-description',
+    );
+  });
+
+  test('renders with a custom data-lgid', () => {
+    const { container } = renderDescription({
+      children: 'Test description',
+      'data-lgid': 'lg-custom-lgid',
+    });
+    expect(container.querySelector('p')).toHaveAttribute(
+      'data-lgid',
+      'lg-custom-lgid',
+    );
+  });
 });

packages/typography/src/Description/Description.tsx

@@ -33,10 +33,12 @@ export const Description = Polymorphic<DescriptionProps>(
     const as = asProp ?? asDerivedFromChildren;
     const { Component } = usePolymorphic(as);
 
+    const lgIds = getLgIds(dataLgId ?? getLgIds().description);
+
     return (
       <Component
-        data-lgid={getLgIds(dataLgId).description}
-        data-testid={getLgIds(dataLgId).description}
+        data-lgid={lgIds.root}
+        data-testid={lgIds.root}
         className={cx(
           getDescriptionStyle(theme),
           descriptionTypeScaleStyles[baseFontSize],

packages/typography/src/Label/Label.tsx

@@ -33,10 +33,12 @@ export const Label = Polymorphic<BaseLabelProps>(
     const baseFontSize = useUpdatedBaseFontSize(baseFontSizeOverride);
     const { Component } = usePolymorphic(as);
 
+    const lgIds = getLgIds(dataLgId ?? getLgIds().label);
+
     return (
       <Component
-        data-lgid={getLgIds(dataLgId).label}
-        data-testid={getLgIds(dataLgId).label}
+        data-lgid={lgIds.root}
+        data-testid={lgIds.root}
         className={cx(
           getLabelStyles(theme),
           labelTypeScaleStyles[baseFontSize],

tools/eslint-plugin/scripts/buildRulesIndex.ts

@@ -30,6 +30,7 @@ export function buildRulesIndexFile() {
     const indexContent = `/**
  * DO NOT MODIFY THIS FILE
  * ANY CHANGES WILL BE REMOVED ON THE NEXT BUILD
+ * USE THE "create-rule" SCRIPT TO ADD NEW RULES INSTEAD
  */
 ${importStatements}
 

tools/eslint-plugin/src/rules/index.ts

@@ -1,11 +1,14 @@
 /**
  * DO NOT MODIFY THIS FILE
  * ANY CHANGES WILL BE REMOVED ON THE NEXT BUILD
+ * USE THE "create-rule" SCRIPT TO ADD NEW RULES INSTEAD
  */
+import { noDuplicateIdsRule } from './no-duplicate-ids';
 import { noIndirectImportsRule } from './no-indirect-imports';
 import { noRenderHookFromRtlRule } from './no-render-hook-from-rtl';
 
 export const rules = {
+  'no-duplicate-ids': noDuplicateIdsRule,
   'no-indirect-imports': noIndirectImportsRule,
   'no-render-hook-from-rtl': noRenderHookFromRtlRule,
 };

tools/eslint-plugin/src/rules/no-duplicate-ids.ts

@@ -0,0 +1,74 @@
+import { TSESTree } from '@typescript-eslint/types';
+
+import { createRule } from '../utils/createRule';
+
+export const noDuplicateIdsRule = createRule({
+  name: 'no-duplicate-ids',
+  meta: {
+    type: 'suggestion',
+    messages: {
+      'issue:noDuplicateIds':
+        'Do not reference the same lgid or testid in multiple places within a component',
+    },
+    schema: [],
+    docs: {
+      description:
+        'Prevents duplicate lgid or testid usage within a component file',
+    },
+  },
+  defaultOptions: [],
+  create: context => {
+    const seen: Record<
+      'data-lgid' | 'data-testid',
+      Record<string, TSESTree.JSXAttribute | undefined>
+    > = {
+      'data-lgid': {},
+      'data-testid': {},
+    };
+
+    return {
+      JSXAttribute(node) {
+        if (node.name.type !== 'JSXIdentifier') {
+          return;
+        }
+
+        const { name: attributeName } = node.name;
+
+        if (attributeName !== 'data-lgid' && attributeName !== 'data-testid') {
+          return;
+        }
+
+        if (
+          node.value?.type !== 'JSXExpressionContainer' ||
+          node.value.expression.type !== 'MemberExpression' ||
+          node.value.expression.object.type !== 'Identifier' ||
+          node.value.expression.object.name !== 'lgIds'
+        ) {
+          return;
+        }
+
+        const { property } = node.value.expression;
+
+        const name =
+          property.type === 'Identifier'
+            ? property.name
+            : property.type === 'Literal'
+            ? property.value
+            : undefined;
+
+        if (typeof name === 'string') {
+          const other = seen[attributeName][name];
+
+          if (other) {
+            context.report({
+              node,
+              messageId: 'issue:noDuplicateIds',
+            });
+          } else {
+            seen[attributeName][name] = node;
+          }
+        }
+      },
+    };
+  },
+});

tools/eslint-plugin/src/tests/no-duplicate-lgids.spec.ts

@@ -0,0 +1,35 @@
+import { noDuplicateIdsRule } from '../rules/no-duplicate-ids';
+
+import { ruleTester } from './utils/ruleTester.testutils';
+
+ruleTester.run('no-duplicate-ids', noDuplicateIdsRule, {
+  valid: [
+    {
+      code: `<div data-lgid={lgIds.root}><div data-lgid={lgIds.nested} /></div>`,
+      languageOptions: {
+        parserOptions: {
+          ecmaFeatures: {
+            jsx: true,
+          },
+        },
+      },
+    },
+  ],
+  invalid: [
+    {
+      code: `<div data-lgid={lgIds.root}><div data-lgid={lgIds.root} /></div>`,
+      languageOptions: {
+        parserOptions: {
+          ecmaFeatures: {
+            jsx: true,
+          },
+        },
+      },
+      errors: [
+        {
+          messageId: 'issue:noDuplicateIds',
+        },
+      ],
+    },
+  ],
+});