PropTypes and Typescript duplication

[elycheea]

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.

@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]

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]

Slack convo excerpts with Taylor.

• We don't think it's a good idea to remove proptypes right now. There are apps out there not using typescript and therefore relying on PropTypes. This is a play to maximize our compatibility.
• If we were to remove them, we would need to gather data that suggests doing so is the right play.
• Removing would qualify as a breaking change.

[tay1orjones]

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.

• Begin generating proptypes from typescript interface(s) instead of manually authoring them
  • babel-plugin-typescript-to-proptypes or something? I'm not sure. MUI has a hefty bit of infrastructure to do this.
• Declare typescript interfaces the "source of truth" instead of proptypes
  • Update documentation (website, storybook, repo md(x))
  • Ensure storybook generates prop tables from TS
• TypeScript types become bound by semver
  • Ensure stability of types via something like the public-api snapshot system
• Proptypes become no longer bound by semver, overall provided on an "as-is" basis
  • Probably could still leave in the proptypes public api snapshot for visibility's-sake

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]

@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]

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]

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.

[tay1orjones]

@wkeese Production builds should use something like https://www.npmjs.com/package/babel-plugin-transform-react-remove-prop-types to strip the proptypes.

I'm not sure if the additional size realistically impacts dx during local development.

[wkeese]

@wkeese Production builds should use something like https://www.npmjs.com/package/babel-plugin-transform-react-remove-prop-types to strip the proptypes.

Yes, you should do that in your production build of Carbon.

But it doesn't matter if I run that plugin in my production build, because that wouldn't affect the code in Carbon. In other words, Babel runs on code in the src/ directory, not in the node_modules/ directory. (I know technically you can configure Babel to run on node_modules/, but it's not practical.)

[tay1orjones]

Yeah, we can't strip them but we could wrap them in a __DEV__ check like we do in @carbon/icons-react so they'll be tree shaken/dead code eliminated. It might help us get to a flat entrypoint at some point, too - carbon-design-system/carbon#5442

[wkeese]

Sounds good although I'm not actually sure what you mean by "flat entrypoint".