Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pigment-css] Fix props forwarding #41688

Merged
merged 15 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/pigment-css-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
*/

/**
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this is comparatively faster than Object.keys since optimizing this styled call is the most important part as it'll be called a lot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, it's the same logic as emotion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styled-component also do the same.

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={{
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
},
},
],
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.b1prasel-1 {
color: tomato;
}
.b1prasel-2 {
color: salmon;
}
Original file line number Diff line number Diff line change
@@ -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',
},
],
});
Loading
Loading