carbon-design-system / ibm-products

A Carbon-powered React component library for IBM Products
https://ibm-products.carbondesignsystem.com
Apache License 2.0
92 stars 136 forks source link

PropTypes and Typescript duplication #4466

Open elycheea opened 6 months ago

elycheea commented 6 months ago

From Feb 27, 2024 retrospective.

Currently, we’re keeping both PropTypes and Typescript, but consider this is also duplication. (Note that Typescript checks type at compile while PropTypes checks during runtime.) Duplication does also mean additional work for us longer term if we add or change props.

Image

@lee-chase brought up that some props have custom validation so would need an alternative in order to remove the prop types for these.

sstrubberg commented 5 months ago

I'd love to have a unified Carbon stance on this topic. We should all get together. @elycheea @jeffchew @tay1orjones @kennylam @matthewgallo @lee-chase

Maybe a topic for our next Dev call?

sstrubberg commented 5 months ago

Slack convo excerpts with Taylor.

tay1orjones commented 5 months ago

If data suggests there is still a large population not using typescript, and we all firmly agree it's too burdensome to maintain both, we could explore a "soft-deprecation" of proptypes for v12.

With the goal of actually removing proptypes in v13 or something. Personally I'm not totally sold on this (cost/reward seems out of balance), but it's something to talk about.

elycheea commented 5 months ago

@sstrubberg @tay1orjones Sounds like next dev call will have some funteresting topics lined up. 😀 We had a few other we wanted to add to the list.

matthewgallo commented 4 months ago

I'm wondering how or if our viewpoint on this changes at all when taking into consideration that prop-types is deprecated and will be officially removed in React 19 (more info here). Their before and after example from the React 19 upgrade guide shows only typescript types.

// Before
import PropTypes from 'prop-types';

function Heading({text}) {
  return <h1>{text}</h1>;
}
Heading.propTypes = {
  text: PropTypes.string,
};
Heading.defaultProps = {
  text: 'Hello, world!',
};

// After
interface Props {
  text?: string;
}
function Heading({text = 'Hello, world!'}: Props) {
  return <h1>{text}</h1>;
}

@tay1orjones I like the idea you posed around possibly generating prop types so that we can maintain support for previous React versions and it would also allow us to remove some of the duplication that we already see in our components.

wkeese commented 1 month ago

The other thing is that having PropTypes bloats the code (i.e. it bloats the output from Webpack). So I understand your desire to be nice to people still using plain Javascript, but it does add a cost to all applications.

Of course this applies to @carbon/react as well as @carbon/ibm-products.