Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Components package: Export Props types as Public API #60193

Closed
mikeybinns opened this issue Mar 26, 2024 · 7 comments · Fixed by #60350
Closed

Components package: Export Props types as Public API #60193

mikeybinns opened this issue Mar 26, 2024 · 7 comments · Fixed by #60350
Assignees
Labels
Developer Experience Ideas about improving block and theme developer experience [Package] Components /packages/components [Status] In Progress Tracking issues with work in progress [Type] Code Quality Issues or PRs that relate to code quality

Comments

@mikeybinns
Copy link
Contributor

What problem does this address?

It's currently difficult to retrieve types for the Props of each element because the prop types are not exported. This can lead to issues like exporting them from subdirectories that may change, or inconsistent ways of retrieving prop types for different components. Prop types are often used, especially when nesting a component within another component.

This issue was initially raised here: DefinitelyTyped/DefinitelyTyped#69133

What is your proposed solution?

Export the prop types as a public API so they can be consumed by projects / libraries.
e.g. if Slot is the component, the props type should be SlotProps and it should be exported from the root.

@mikeybinns mikeybinns added the [Type] Enhancement A suggestion for improvement. label Mar 26, 2024
@mikeybinns mikeybinns changed the title Export Props types as Public API Components package: Export Props types as Public API Mar 26, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Mar 27, 2024
@gziolo gziolo added Developer Experience Ideas about improving block and theme developer experience [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components and removed [Type] Enhancement A suggestion for improvement. labels Mar 28, 2024
@mirka
Copy link
Member

mirka commented Mar 28, 2024

We've actually been trying to avoid having to explicitly export prop types, in order to reduce maintenance cost. I do understand that a lot of component libraries do export types though, and we do want to support everybody's use cases.

Are there any specific use cases you've encountered that cannot be covered by using utility types like React.ComponentProps< typeof Button > to extract the prop types directly from the component? Even internally, we're using these utility types a lot.

@rafaucau
Copy link

rafaucau commented Mar 28, 2024

It seems that React.ComponentProps does not work with components that use forwadRef: DefinitelyTyped/DefinitelyTyped#69133 (comment)

@mirka
Copy link
Member

mirka commented Mar 28, 2024

@rafaucau Do you have a sandbox repro that we could look at? It seems to be working as expected for me, but maybe I'm missing something specific in your use case.

@mikeybinns
Copy link
Contributor Author

@mirka I had forgotten about React.ComponentProps, so potentially it could work (as long as @rafaucau's issue is resolved)

I do have two points if this is how we intend to move forward:

  1. The most common / industry defacto standard way to get props types is to import them from the package as a type. If we're not going to do that, we should document in the package README file how to get props types from this package.
  2. ComponentProps is not exported from @wordpress/element which is where you're supposed to import react things from in WP projects, so this should be fixed so that people don't need to import from react directly.

Also, as a compromise, I could ditch my current PR, and instead of changing all the individual component files to export props manually, I could instead just export types using ComponentProps, like this:

./angle-picker-control/index.ts

export default AnglePickerControl; // already present
export type Props = React.ComponentProps< typeof AnglePickerControl >;

./index.ts
export {
	default as AnglePickerControl,
	type Props as AnglePickerControlProps,
} from './angle-picker-control';

This makes it a really simple and low maintenance way to export the props for components. The only maintenance I could see after implementing this is ensuring this is added for all new components that are added.

@mirka
Copy link
Member

mirka commented Mar 28, 2024

@mikeybinns Agreed on both points. I'll note that we've pivoted to recommend direct usage of React (#54074), but I have no issue against adding more type exports to @wordpress/element.

I also think your compromise suggestion is reasonable, and could be a minimal maintenance way to export types if it comes to it, though I still personally hope we won't have to do that. I prefer that we start with encouraging people to use React.ComponentProps and see if that covers everyone's use cases. (For what it's worth, even as a consumer I prefer using React.ComponentProps because it's simpler and less error-prone than having to import a separate type.)

@mikeybinns
Copy link
Contributor Author

mikeybinns commented Apr 1, 2024

@mirka I've added a small PR adding the discussed documentation to the components README file. In the examples, I used a direct import form React due to that ticket you mentioned.

I wasn't aware of the pivot from using @wordpress/element, so I'm happy to not add the import to that package, and with this documentation added, I think we can call this issue resolved :)

@rafaucau
Copy link

rafaucau commented Apr 2, 2024

Do you have a sandbox repro that we could look at? It seems to be working as expected for me, but maybe I'm missing something specific in your use case.

It turns out it works in VSCode, but not in JetBrains IDEs: DefinitelyTyped/DefinitelyTyped#69133 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Package] Components /packages/components [Status] In Progress Tracking issues with work in progress [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
4 participants