Description
When passing props like the leadingIcon and trailingIcon props on a button, it's not uncommon to pass something that relates to props from the containing function.
Currently, to do this one needs to wrap this in a function
<Button leadingIcon={() => getAnIconBasedOnSomething(props)} />This leads to Button needing to render more often than necessary
we could always memoize this, of course
<Button leadingIcon={useCallback(() => getAnIconBasedOnSomething(prop1, prop2), [prop1, prop2])} />But react already has a built in mechanism for dealing with this, components.
<Button leadingIcon={<IconBasedOnSomething prop={prop} />} />This manages these icons, and similar (I assume in other components) in an internal way, similar to children by default, allows the parent context to control what gets rendered, rather than internalizing that to the button, and avoids the need to either bloat code by memoizing what are effectively inline components or handling these inefficiently by default
react/src/Button/ButtonBase.tsx
Lines 29 to 42 in c3eedb2
could be minimally changed to support that
const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
({children, as: Component = 'button', sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
const {leadingIcon, trailingIcon, variant = 'default', size = 'medium'} = props
const {theme} = useTheme()
const baseStyles = useMemo(() => {
return merge.all([getButtonStyles(theme), getSizeStyles(size, variant, false), getVariantStyles(variant, theme)])
}, [theme, size, variant])
const sxStyles = useMemo(() => {
return merge(baseStyles, sxProp as SxProp)
}, [baseStyles, sxProp])
return (
<StyledButton as={Component} sx={sxStyles} {...props} ref={forwardedRef}>
{leadingIcon && (
<Box as="span" data-component="leadingIcon" sx={iconWrapStyles}>
{leadingIcon}
</Box>
)}
{children && <span data-component="text">{children}</span>}
{trailingIcon && (
<Box as="span" data-component="trailingIcon" sx={trailingIconStyles}>
{trailingIcon}
</Box>
)}
</StyledButton>
)
}
) as PolymorphicForwardRefComponent<'button' | 'a', ButtonProps>with no loss internally and more power for consumers to render what/how they need to
this would make the simple case slightly more complex - by default
<Button leadingIcon={LeadingIcon} />we could choose to handle this internally, but checking whether react-is.isComponentTypes (any of them), and handling them separately. I don't think we should, because this is a moving target of possibility - prone to error, and the difference is minimal to consumers who are used to writing JSX when dealing with components
Components rendered this way could be memo forwardRef lazy or more, none of which are matches for the FunctionComponent interface currently, and all implement a standard that considers them React.ReactNodes