[pigment-css] Fix props forwarding (#41688)

mui · Mar 28, 2024 · e9db5b8 · e9db5b8
1 parent 933b6f4
commit e9db5b8
Show file tree

Hide file tree

Showing 9 changed files with 328 additions and 62 deletions.
diff --git a/packages/pigment-css-react/package.json b/packages/pigment-css-react/package.json
@@ -37,6 +37,7 @@
     "@babel/parser": "^7.23.9",
     "@babel/types": "^7.23.9",
     "@emotion/css": "^11.11.2",
+    "@emotion/is-prop-valid": "^1.2.2",
     "@emotion/react": "^11.11.4",
     "@emotion/serialize": "^1.1.3",
     "@emotion/styled": "^11.11.0",

diff --git a/packages/pigment-css-react/src/styled.jsx → packages/pigment-css-react/src/styled.js b/packages/pigment-css-react/src/styled.jsx → packages/pigment-css-react/src/styled.js
@@ -1,5 +1,6 @@
 import * as React from 'react';
 import clsx from 'clsx';
+import isPropValid from '@emotion/is-prop-valid';
 
 function getVariantClasses(componentProps, variants) {
   const { ownerState = {} } = componentProps;
@@ -15,16 +16,21 @@ function getVariantClasses(componentProps, variants) {
   return variantClasses;
 }
 
-/**
- * @param {string} propKey
- * @returns {boolean}
- */
-function defaultShouldForwardProp(propKey) {
-  return propKey !== 'sx' && propKey !== 'as' && propKey !== 'ownerState';
+function isHtmlTag(tag) {
+  return (
+    typeof tag === 'string' &&
+    // 96 is one less than the char code
+    // for "a" so this is checking that
+    // it's a lowercase character
+    tag.charCodeAt(0) > 96
+  );
 }
 
+const slotShouldForwardProp = (key) => key !== 'sx' && key !== 'as' && key !== 'ownerState';
+const rootShouldForwardProp = (key) => slotShouldForwardProp(key) && key !== 'classes';
+
 /**
- * @typedef {typeof defaultShouldForwardProp} ShouldForwardProp
+ * @typedef {(propKey: string) => boolean} ShouldForwardProp
  */
 
 /**
@@ -40,10 +46,20 @@ function defaultShouldForwardProp(propKey) {
  * @param {Object} componentMeta.defaultProps Default props object copied over and inlined from theme object
  */
 export default function styled(tag, componentMeta = {}) {
-  const { name, slot, shouldForwardProp = defaultShouldForwardProp } = componentMeta;
+  const { name, slot, shouldForwardProp } = componentMeta;
+
+  let finalShouldForwardProp = shouldForwardProp;
+  if (!shouldForwardProp) {
+    if (isHtmlTag(tag)) {
+      finalShouldForwardProp = isPropValid;
+    } else if (slot === 'Root' || slot === 'root') {
+      finalShouldForwardProp = rootShouldForwardProp;
+    } else {
+      finalShouldForwardProp = slotShouldForwardProp;
+    }
+  }
+  const shouldUseAs = !finalShouldForwardProp('as');
   /**
-   * @TODO - Filter props and only pass necessary props to children
-   *
    * This is the runtime `styled` function that finally renders the component
    * after transpilation through WyW-in-JS. It makes sure to add the base classes,
    * variant classes if they satisfy the prop value and also adds dynamic css
@@ -62,26 +78,10 @@ export default function styled(tag, componentMeta = {}) {
    */
   function scopedStyledWithOptions(options = {}) {
     const { displayName, classes = [], vars: cssVars = {}, variants = [] } = options;
-    let componentName = 'Component';
-
-    if (name) {
-      if (slot) {
-        componentName = `${name}${slot}`;
-      } else {
-        componentName = name;
-      }
-    } else if (displayName) {
-      componentName = displayName;
-    }
 
-    const StyledComponent = React.forwardRef(function StyledComponent(
-      // eslint-disable-next-line react/prop-types
-      { as, className, sx, style, ...props },
-      ref,
-    ) {
-      // eslint-disable-next-line react/prop-types
-      const { ownerState, ...restProps } = props;
-      const Component = as ?? tag;
+    const StyledComponent = React.forwardRef(function StyledComponent(inProps, ref) {
+      const { as, className, sx, style, ownerState, ...props } = inProps;
+      const Component = (shouldUseAs && as) || tag;
       const varStyles = Object.entries(cssVars).reduce(
         (acc, [cssVariable, [variableFunction, isUnitLess]]) => {
           const value = variableFunction(props);
@@ -97,9 +97,7 @@ export default function styled(tag, componentMeta = {}) {
         },
         {},
       );
-      // eslint-disable-next-line react/prop-types
       const sxClass = typeof sx === 'string' ? sx : sx?.className;
-      // eslint-disable-next-line react/prop-types
       const sxVars = sx && typeof sx !== 'string' ? sx.vars : undefined;
 
       if (sxVars) {
@@ -112,40 +110,31 @@ export default function styled(tag, componentMeta = {}) {
         });
       }
 
-      const finalClassName = clsx(classes, sxClass, className, getVariantClasses(props, variants));
-      const toPassProps = Object.keys(restProps)
-        .filter((item) => {
-          const res = shouldForwardProp(item);
-          if (res) {
-            return defaultShouldForwardProp(item);
-          }
-          return false;
-        })
-        .reduce((acc, key) => {
-          acc[key] = restProps[key];
-          return acc;
-        }, {});
+      const finalClassName = clsx(
+        classes,
+        sxClass,
+        className,
+        getVariantClasses(inProps, variants),
+      );
+
+      const newProps = {};
+      // eslint-disable-next-line no-restricted-syntax
+      for (const key in props) {
+        if (shouldUseAs && key === 'as') {
+          continue;
+        }
 
-      // eslint-disable-next-line no-underscore-dangle
-      if (!Component.__isStyled || typeof Component === 'string') {
-        return (
-          // eslint-disable-next-line react/jsx-filename-extension
-          <Component
-            {...toPassProps}
-            ref={ref}
-            className={finalClassName}
-            style={{
-              ...varStyles,
-              ...style,
-            }}
-          />
-        );
+        if (finalShouldForwardProp(key)) {
+          newProps[key] = props[key];
+        }
       }
 
       return (
         <Component
-          {...toPassProps}
-          ownerState={ownerState}
+          {...newProps}
+          // pass down `ownerState` to nested styled components
+          // eslint-disable-next-line no-underscore-dangle
+          {...(Component.__styled_by_pigment_css && { ownerState })}
           ref={ref}
           className={finalClassName}
           style={{
@@ -156,9 +145,13 @@ export default function styled(tag, componentMeta = {}) {
       );
     });
 
+    let componentName = displayName;
+    if (!componentName && name) {
+      componentName = `${name}${slot ? `-${slot}` : ''}`;
+    }
     StyledComponent.displayName = `Styled(${componentName})`;
     // eslint-disable-next-line no-underscore-dangle
-    StyledComponent.__isStyled = true;
+    StyledComponent.__styled_by_pigment_css = true;
 
     return StyledComponent;
   }

diff --git a/packages/pigment-css-react/tests/styled/fixtures/styled-variants.input.js b/packages/pigment-css-react/tests/styled/fixtures/styled-variants.input.js
@@ -0,0 +1,18 @@
+import { styled } from '@pigment-css/react';
+
+const Button = styled('button')({
+  variants: [
+    {
+      props: { color: 'primary' },
+      style: {
+        color: 'tomato',
+      },
+    },
+    {
+      props: ({ ownerState }) => ownerState.color === 'secondary',
+      style: {
+        color: 'salmon',
+      },
+    },
+  ],
+});
diff --git a/packages/pigment-css-react/tests/styled/fixtures/styled-variants.output.css b/packages/pigment-css-react/tests/styled/fixtures/styled-variants.output.css
@@ -0,0 +1,6 @@
+.b1prasel-1 {
+  color: tomato;
+}
+.b1prasel-2 {
+  color: salmon;
+}
diff --git a/packages/pigment-css-react/tests/styled/fixtures/styled-variants.output.js b/packages/pigment-css-react/tests/styled/fixtures/styled-variants.output.js
@@ -0,0 +1,17 @@
+import { styled as _styled } from '@pigment-css/react';
+import _theme from '@pigment-css/react/theme';
+const Button = /*#__PURE__*/ _styled('button')({
+  classes: ['b1prasel'],
+  variants: [
+    {
+      props: {
+        color: 'primary',
+      },
+      className: 'b1prasel-1',
+    },
+    {
+      props: ({ ownerState }) => ownerState.color === 'secondary',
+      className: 'b1prasel-2',
+    },
+  ],
+});