system-ui / theme-ui

Build consistent, themeable React apps based on constraint-based design principles
https://theme-ui.com
MIT License
5.3k stars 673 forks source link

Forwarding refs for a polymorphic React component #2356

Open SaraRavasio opened 2 years ago

SaraRavasio commented 2 years ago

Is your feature request related to a problem? Please describe.

At the moment, if I would try to use Box with a as='svg' viewBox='0 0 24 24' property, typescript will complain about the fact that viewBox is not a Box property, but that shouldn't be the case, as it is a property of svg. Plus, they should also allow passing and accepting the correct ref property.

image

The same goes for other components, especially Flex and Text.

I did my own local override implementation of the types taking inspiration from this article, but would be great if this would come directly from theme-ui.

import React from 'react';
import { Box as ThemeUIBox, BoxProps as ThemeUIBoxProps } from 'theme-ui';

import {
  PolymorphicComponentPropsWithRef,
  PolymorphicRef,
} from '../../../util/types';

export type BoxProps<C extends React.ElementType = 'div'> =
  PolymorphicComponentPropsWithRef<C, ThemeUIBoxProps>;

type BoxComponent = <C extends React.ElementType = 'div'>(
  props: BoxProps<C>
) => React.ReactElement | null;

export const Box: BoxComponent = React.forwardRef(
  <C extends React.ElementType = 'div'>(
    props: BoxProps<C>,
    ref?: PolymorphicRef<C>
  ) => <ThemeUIBox ref={ref} {...props} />
);

Thank you

lachlanjc commented 2 years ago

Thanks for the detailed issue! PR fixing this for everyone would be very welcomed :) Previous attempts to fix this have been too slow, unfortunately.

hasparus commented 2 years ago

Just FYI, the PR addressing this should attach tsc --diagnostics.

Personally, I think it's a footgun, and its every use case could be instead built with sx prop handled by JSX pragma. TypeScript can handle your JSX pragmas, so this shouldn't be a hard sell.

Radix UI's polymorphic package was pretty good, yet they still deprecated it in favor of asChild prop.