ESLint rule to validate css props are declared before{...spread} …

…props (elastic#6257) * WIP. First pass at CSS before spread child props ESLint rule. * Refactoring CSS and spread child prop index calls. * Improving the error reporting message. * Renaming file for clarity. Adding first passing unit tests. * Focused on JSXElement, added unit tests, fixed failures in local linting." * Updating unit tests to pass before renaming. * Registering ESLint rule locally before adding unit tests. * Switched to JSXElement, and array of arrays. * Simplified logic, added unit tests, fixed failures. * Renamed temp arrays and refactored early returns for clarity. * Removing contentProps.css from cssContentStyles array.
cee-chen · Sep 28, 2022 · 9d75128 · 9d75128
1 parent e11f280
commit 9d75128
Show file tree

Hide file tree

Showing 12 changed files with 304 additions and 10 deletions.
diff --git a/.eslintplugin.js b/.eslintplugin.js
@@ -4,4 +4,5 @@ exports.rules = {
   'require-license-header': require('./scripts/eslint-plugin/require_license_header'),
   'forward-ref': require('./scripts/eslint-plugin/forward_ref_display_name'),
   'css-logical-properties': require('./scripts/eslint-plugin/css_logical_properties'),
+  'css_before_spread_props': require('./scripts/eslint-plugin/css_before_spread_props'),
 };
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -45,6 +45,7 @@ module.exports = {
     'local/href-with-rel': 'error',
     'local/forward-ref': 'error',
     'local/css-logical-properties': 'error',
+    'local/css_before_spread_props': 'error',
     'local/require-license-header': [
       'warn',
       {

diff --git a/scripts/eslint-plugin/css_before_spread_props.js b/scripts/eslint-plugin/css_before_spread_props.js
@@ -0,0 +1,48 @@
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Enforce CSS props are declared before spread props',
+    },
+  },
+  create: function (context) {
+    const allElementsArr = [];
+
+    return {
+      JSXElement(node) {
+        const attributesArr = [];
+        node.openingElement.attributes.forEach((attribute) =>
+          attributesArr.push(attribute)
+        );
+        allElementsArr.push(attributesArr);
+      },
+      'Program:exit'() {
+        allElementsArr.forEach((elementArr) => {
+          const cssPropsIndex = elementArr.findIndex(
+            (node) => node.name && node.name.name === 'css'
+          );
+          if (cssPropsIndex === -1) return;
+
+          const spreadPropsIndex = elementArr.findIndex(
+            (node) => node.type === 'JSXSpreadAttribute'
+          );
+          if (spreadPropsIndex === -1) return;
+
+          if (cssPropsIndex > spreadPropsIndex) {
+            context.report({
+              loc: {
+                line: elementArr[cssPropsIndex].loc.start.line,
+                column: elementArr[cssPropsIndex].loc.start.column,
+              },
+              message:
+                '{{ identifier }}: CSS props must be declared before spread props.',
+              data: {
+                identifier: elementArr[spreadPropsIndex].argument.name,
+              },
+            });
+          }
+        });
+      },
+    };
+  },
+};
diff --git a/scripts/eslint-plugin/css_before_spread_props.test.js b/scripts/eslint-plugin/css_before_spread_props.test.js
@@ -0,0 +1,245 @@
+import rule from './css_before_spread_props.js';
+const RuleTester = require('eslint').RuleTester;
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('babel-eslint'),
+});
+
+const valid = [
+  {
+    code: `<a
+        css={cssStyles}
+        {...rest}
+      >
+        {buttonIcon}
+      </a>
+    `,
+  },
+  {
+    code: `<a css={cssStyles}>
+        {buttonIcon}
+      </a>
+    `,
+  },
+  {
+    code: `<a {...rest}>
+        {buttonIcon}
+      </a>
+    `,
+  },
+  {
+    code: `<nav {...rest}>
+      <ol
+        className="euiBreadcrumbs__list"
+        css={cssBreadcrumbsListStyles}
+      >
+        {breadcrumbChildren}
+      </ol>
+    </nav>
+    `,
+  },
+  {
+    code: `<nav {...rest}>
+      <ol css={cssBreadcrumbsListStyles}>
+        {breadcrumbChildren}
+      </ol>
+    </nav>
+    `,
+  },
+  {
+    code: `<nav {...rest}>
+      <ol {...rest}>
+        {breadcrumbChildren}
+      </ol>
+    </nav>
+    `,
+  },
+  {
+    code: `<EuiLink
+      ref={ref}
+      css={cssStyles}
+      {...rest}
+    >
+      {text}
+    </EuiLink>
+    `,
+  },
+  {
+    code: `<EuiLink
+      ref={ref}
+      css={cssStyles}
+      {...rest}
+    >
+      Text string
+    </EuiLink>
+    `,
+  },
+];
+
+const invalid = [
+  {
+    code: `<a
+      {...rest}
+      css={cssStyles}
+    >
+      {buttonIcon}
+    </a>
+    `,
+    errors: [
+      {
+        message: 'rest: CSS props must be declared before spread props.',
+      },
+    ],
+  },
+  {
+    code: `<nav {...rest}>
+      <ol
+        {...rest}
+        css={cssBreadcrumbsListStyles}
+      >
+        {breadcrumbChildren}
+      </ol>
+    </nav>
+    `,
+    errors: [
+      {
+        message: 'rest: CSS props must be declared before spread props.',
+      },
+    ],
+  },
+  {
+    code: `<nav>
+      <ol
+        {...rest}
+        css={cssBreadcrumbsListStyles}
+      >
+        {breadcrumbChildren}
+      </ol>
+    </nav>
+    `,
+    errors: [
+      {
+        message: 'rest: CSS props must be declared before spread props.',
+      },
+    ],
+  },
+  {
+    code: `<EuiLink
+      ref={ref}
+      {...rest}
+      css={cssStyles}
+    >
+      {text}
+    </EuiLink>
+    `,
+    errors: [
+      {
+        message: 'rest: CSS props must be declared before spread props.',
+      },
+    ],
+  },
+  {
+    code: `<EuiLink
+      ref={ref}
+      {...rest}
+      css={cssStyles}
+    >
+      Text string
+    </EuiLink>
+    `,
+    errors: [
+      {
+        message: 'rest: CSS props must be declared before spread props.',
+      },
+    ],
+  },
+  {
+    code: `return !href && !onClick ? (
+      <EuiTextColor
+        color={highlightLastBreadcrumb ? 'default' : 'subdued'}
+        cloneElement
+        {...restTextColor}
+        css={cssStyles}
+      >
+        <span
+          ref={ref}
+          title={title}
+          aria-current={ariaCurrent}
+          className={classes}
+          css={cssStyles}
+        >
+          {text}
+        </span>
+      </EuiTextColor>
+    ) : (
+      <EuiLink
+        {...restLink}
+        ref={ref}
+        title={title}
+        aria-current={ariaCurrent}
+        className={classes}
+        css={cssStyles}
+        color={color || (highlightLastBreadcrumb ? 'text' : 'subdued')}
+        onClick={onClick}
+        href={href}
+        rel={rel}
+      >
+        {text}
+      </EuiLink>
+    );
+    `,
+    errors: [
+      {
+        message: 'restTextColor: CSS props must be declared before spread props.',
+      },
+      {
+        message: 'restLink: CSS props must be declared before spread props.',
+      },
+    ],
+  },
+  {
+    code: `return !href && !onClick ? (
+      <EuiTextColor
+        color={highlightLastBreadcrumb ? 'default' : 'subdued'}
+        cloneElement
+        css={cssStyles}
+      >
+        <span
+          ref={ref}
+          title={title}
+          aria-current={ariaCurrent}
+          className={classes}
+          {...restSpanProps}
+          css={cssStyles}
+        >
+          {text}
+        </span>
+      </EuiTextColor>
+    ) : (
+      <EuiLink
+        ref={ref}
+        title={title}
+        aria-current={ariaCurrent}
+        className={classes}
+        css={cssStyles}
+        color={color || (highlightLastBreadcrumb ? 'text' : 'subdued')}
+        onClick={onClick}
+        href={href}
+        rel={rel}
+      >
+        {text}
+      </EuiLink>
+    );
+    `,
+    errors: [
+      {
+        message: 'restSpanProps: CSS props must be declared before spread props.',
+      },
+    ],
+  },
+]
+
+ruleTester.run('css_before_spread_props', rule, {
+  valid,
+  invalid,
+});
diff --git a/src/components/flyout/flyout_body.tsx b/src/components/flyout/flyout_body.tsx
@@ -42,7 +42,7 @@ export const EuiFlyoutBody: EuiFlyoutBodyProps = ({
   ];
 
   return (
-    <div className={classes} {...rest} css={cssStyles}>
+    <div className={classes} css={cssStyles} {...rest}>
       <div
         tabIndex={0}
         className="euiFlyoutBody__overflow"

diff --git a/src/components/flyout/flyout_footer.tsx b/src/components/flyout/flyout_footer.tsx
@@ -29,7 +29,7 @@ export const EuiFlyoutFooter: EuiFlyoutFooterProps = ({
   const cssStyles = [styles.euiFlyoutFooter];
 
   return (
-    <div className={classes} {...rest} css={cssStyles}>
+    <div className={classes} css={cssStyles} {...rest}>
       {children}
     </div>
   );

diff --git a/src/components/flyout/flyout_header.tsx b/src/components/flyout/flyout_header.tsx
@@ -33,7 +33,7 @@ export const EuiFlyoutHeader: EuiFlyoutHeaderProps = ({
   const cssStyles = [styles.euiFlyoutHeader, hasBorder && styles.hasBorder];
 
   return (
-    <div className={classes} {...rest} css={cssStyles}>
+    <div className={classes} css={cssStyles} {...rest}>
       {children}
     </div>
   );

diff --git a/src/components/image/image_fullscreen_wrapper.tsx b/src/components/image/image_fullscreen_wrapper.tsx
@@ -70,9 +70,9 @@ export const EuiImageFullScreenWrapper: FunctionComponent<EuiImageWrapperProps>
         <>
           <figure
             aria-label={optionalCaptionText}
+            css={cssStyles}
             {...wrapperProps}
             className={classes}
-            css={cssStyles}
           >
             <EuiImageButton
               hasAlt={!!alt}

diff --git a/src/components/page/page_section/page_section.tsx b/src/components/page/page_section/page_section.tsx
@@ -100,12 +100,11 @@ export const EuiPageSection: FunctionComponent<EuiPageSectionProps> = ({
     bottomBorder === true && styles.border,
     alignment.toLowerCase().includes('center') && contentStyles.center,
     restrictWidth && contentStyles.restrictWidth,
-    contentProps?.css && contentProps.css,
   ];
 
   return (
     <Component css={cssStyles} {...rest}>
-      <div {...contentProps} css={cssContentStyles} style={widthStyles}>
+      <div css={cssContentStyles} {...contentProps} style={widthStyles}>
         {children}
       </div>
     </Component>

diff --git a/src/components/pagination/pagination.tsx b/src/components/pagination/pagination.tsx
@@ -292,9 +292,9 @@ export const EuiPagination: FunctionComponent<Props> = ({
 
       centerPageCount = (
         <ul
-          {...accessibleName}
-          css={paginationStyles.euiPagination__list}
           className="euiPagination__list"
+          css={paginationStyles.euiPagination__list}
+          {...accessibleName}
         >
           {firstPageButtons}
           {selectablePages}

diff --git a/src/components/tool_tip/tool_tip_anchor.tsx b/src/components/tool_tip/tool_tip_anchor.tsx
@@ -56,8 +56,8 @@ export const EuiToolTipAnchor = forwardRef<
       // eslint-disable-next-line jsx-a11y/mouse-events-have-key-events
       <span
         ref={ref}
-        {...rest}
         css={cssStyles}
+        {...rest}
         className={classes}
         onMouseOver={onMouseOver}
         onMouseOut={onMouseOut}

diff --git a/src/services/theme/clone_element.test.tsx b/src/services/theme/clone_element.test.tsx
@@ -63,7 +63,7 @@ describe('cloneElementWithCss', () => {
 
   it('handles components', () => {
     const TestComponent: React.FC = (props) => (
-      <div {...props} css={{ backgroundColor: 'blue' }}>
+      <div css={{ backgroundColor: 'blue' }} {...props}>
         hello world
       </div>
     );