-
Notifications
You must be signed in to change notification settings - Fork 33
fix: Fixing ComponentType props #2498
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
Conversation
packages/plugin/src/PluginTypes.ts
Outdated
export type ElementName = string; | ||
|
||
/** A mapping of element names to their React components. */ | ||
export type ElementPluginMappingDefinition = Record< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these two types it'd be better to type the props with a Generic. See how we do it ComponentUtils, but would be something like this:
export type ElementPluginMappingDefinition<P extends Record<string, unknown> = {}> = Record<ElementName, React.ComponentType<P>>;
We get better typing that way when using it, and we don't need to use any
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've got something generic working now
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2498 +/- ##
=======================================
Coverage 44.67% 44.67%
=======================================
Files 762 762
Lines 42617 42617
Branches 10710 10710
=======================================
Hits 19038 19038
Misses 23568 23568
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as you've tried this in your template and it's working then I think we're good.
Yep I don't see any errors with the generated template |
Part of work for DH-19416 and a followup from #2477
If no props are specified for
React.ComponentType
it defaults to{}
, allowing no props. We need to specify that we can have any.