carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.59k stars 1.76k forks source link

[Feature Request]: Export props for TypeScript components #16154

Open Sam-Wool opened 2 months ago

Sam-Wool commented 2 months ago

The problem

In the @carbon/react project multiple components which have been converted into TypeScript in Carbon 11 are lacking either exports on their props,. Without access to the props, it is much more difficult to create wrappers for components, pass along props as part of parent components, and assigning types to objects that can help build out components neatly. This has affected my team's efforts to move over from Carbon 10 to 11.

For example, in my team's product we leverage the base component's props (aliased here as CarbonAccordionProps) to create the typings for a wrapper for the Accordion component with a simple overwrite. Without these props, we would have to redefine all of the parameters in our codebase by hand, which would open us up to issues down the road if the Carbon component's props change.

image

Included here is a list of all props that are missing exports that my team have found. Note that there may be additional components with this issue that we haven't identified yet.

- [ ] https://github.com/carbon-design-system/carbon/issues/16364
- [ ] https://github.com/carbon-design-system/carbon/issues/16365
- [ ] https://github.com/carbon-design-system/carbon/issues/16366
- [ ] https://github.com/carbon-design-system/carbon/issues/16367
- [ ] https://github.com/carbon-design-system/carbon/issues/16368
- [ ] https://github.com/carbon-design-system/carbon/issues/16369
- [ ] https://github.com/carbon-design-system/carbon/issues/16370
- [ ] https://github.com/carbon-design-system/carbon/issues/16371
- [ ] https://github.com/carbon-design-system/carbon/issues/16372
- [ ] https://github.com/carbon-design-system/carbon/issues/16373
- [ ] https://github.com/carbon-design-system/carbon/issues/16374
- [ ] https://github.com/carbon-design-system/carbon/issues/16375
- [ ] https://github.com/carbon-design-system/carbon/issues/16376
- [ ] https://github.com/carbon-design-system/carbon/issues/16377
- [ ] https://github.com/carbon-design-system/carbon/issues/16378
- [ ] https://github.com/carbon-design-system/carbon/issues/16379
- [ ] https://github.com/carbon-design-system/carbon/issues/16380
- [ ] https://github.com/carbon-design-system/carbon/issues/16381
- [ ] https://github.com/carbon-design-system/carbon/issues/16382
- [ ] https://github.com/carbon-design-system/carbon/issues/16383
- [ ] https://github.com/carbon-design-system/carbon/issues/16384
- [x] TileGroupProps (completed via #16384)
- [ ] https://github.com/carbon-design-system/carbon/issues/16436

We've also noticed some components are missing an index.ts or .d.ts file - this has caused a similar issue when trying to import props belonging to that component. Below is a list of the components we've found with this issue:

- [x] MultiSelect (completed via #16147)
- [ ] https://github.com/carbon-design-system/carbon/issues/16386
- [ ] https://github.com/carbon-design-system/carbon/issues/16385
- [ ] https://github.com/carbon-design-system/carbon/issues/16444
- [ ] https://github.com/carbon-design-system/carbon/issues/16547

The solution

To fix this, we need to export the props (see attached for example of an interface lacking an export).

image

There are also some instances where a prop is missing an index.ts or .d.ts file - in those cases, we should be able to fix this issue by refactoring an existing index.js to index.ts. See below for an example of a healthy index.ts file.

image

Examples

No response

Application/PAL

IBM Storage Fusion

Business priority

Medium Priority = upcoming release but is not pressing

Available extra resources

No response

Code of Conduct

github-actions[bot] commented 2 months ago

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

tay1orjones commented 2 months ago

Thanks for opening this, I think it's worthwhile to consider exporting these. I'm not sure if there's any downsides, but if I'm not mistaken, consumers have always been able to pluck proptypes off the exported components. Makes sense to enable the same through TypeScript.

This would be an excellent opportunity for a contribution from the community.

There are also some instances where a prop is missing an index.ts or .d.ts file

Majority of these cases are due to a sibling component not yet being migrated. Once all exports in the component(s) index.js have been migrated, index.js can be renamed to index.ts and then the .d.ts file will start being automatically generated.

If you notice any that have all their exports migrated, but it's still index.js, you can open a PR or an issue to track those individually. We have a couple of these issues already. Also all this should be fully resolved once everything has types, #12513, which we're really close to completing.

github-actions[bot] commented 2 months ago

The Carbon team has accepted this proposal! Our team doesn't have the capacity to work on this now, so we are requesting community contributors. Please see the labels for roles that are needed. If you are willing to help out, comment below and we will get in touch!

gillesdandrea commented 2 months ago

As a work-around, in typescript it's possible to extract the type of a parameter of a function. So

type MyExtractedProps = Parameters<typeof MyFunctionalComponent>[0];

allow to use the Props of a functional components even if not exported.

wkeese commented 2 months ago

I was going to suggest the same thing, except with ComponentProps, ex:

ComponentProps<typeof TableToolbarSearch>

It's a bit easier to use since you don't need the [0], and IIRC it works for old-school class components too.

Having said that, I have no objection to exporting all the properties explicitly again. That's how it worked in Carbon 10.

tay1orjones commented 2 months ago

@Sam-Wool What do your type imports usually look like? Could you confirm that you're not able to import ModalProps? It's currently exported. It isn't exported from the root but is in the Modal module.

// So I believe this won't work:
import { type ModalProps } from "@carbon/react";

// whereas this should work
import { type ModalProps } from "@carbon/react/es/components/Modal";

I don't know that we'll be able to export these from the root until everything is converted and the root index.js goes to index.tsx and can export types.

Sam-Wool commented 2 months ago

@tay1orjones Our import for ModalProps looks like the former. Did a little testing and it looks like we're able to import it from the Modal module like you showed (see attached) - thanks for pointing that out!

image

tay1orjones commented 2 months ago

Awesome! One more point to clarify, if a component has been converted to .tsx but the index.js for the component is not yet able to be .ts, the exported types/interfaces should still be available at a fully qualified path that bypasses the index.js. Right now MultiSelect is a good example of this:

import { type MultiSelectProps } from "@carbon/react/es/components/MultiSelect/MultiSelect";
tay1orjones commented 1 month ago

We've also noticed some components are missing an index.ts or .d.ts file - this has caused a similar issue when trying to import props belonging to that component. Below is a list of the components we've found with this issue:

  • [ ] CopyButton
  • [ ] MultiSelect
  • [ ] PasswordInput
  • [ ] TileGroup

I've opened a new issue to contain this work:

mbarrer commented 1 month ago

@tay1orjones with regards to ModalProps, this should be easily raised to the @carbon/react bucket if you exported it as well from the /Modal/index.ts bucket. Since the main index.ts of carbon uses the export * from pattern it will pick up those type exports and add them to the bucket.

We've been working a lot this past week on moving things over to carbon11, writing ambient modules to help ease the transition and fill in the missing prop exports. The team was wondering what the expectation is for the end goal of the prop exports. Should we ultimately expect to be able to pull types from the main @carbon/react bucket or should we expect to grab typings from the component files directly?