Icon component: Add types #26216
Icon component: Add types #26216
Conversation
sirreal
added
[Type] Code Quality
sirreal
force-pushed
the
update/type-component-icon
branch
from
c091772 to
d490518
Compare
|
Size Change: +203 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
sirreal
force-pushed
the
update/type-component-icon
branch
from
d490518 to
616d2bf
Compare
| /* eslint-disable jsdoc/valid-types */ | ||
| /** | ||
| * @template T | ||
| * @typedef {T extends import('react').ComponentType<infer U> ? U : T extends string ? import('react').ComponentPropsWithoutRef<'span'> : {}} AdditionalProps |
There was a problem hiding this comment.
Should the final default be never rather than {}? When would we end up in that final branch?
Also an aside, the formatting like this is so frustrating to read... I'm assuming there's no way to split this across multiple lines with JSDoc right? Is that another limitation we could add to the list of reasons why full TS is preferred?
There was a problem hiding this comment.
Should the final default be
neverrather than{}? When would we end up in that final branch?
🤔 This probably isn't perfect, I don't think it captures the case where icon is an element but this is a good start.
We want something that we can intersect (&) with another type to just keep the other type (an identity element). That's {} (I don't know whether there are other types that would work) where A & {} is the same as A. A & never is never (I had to play a bit to test this out).
Also an aside, the formatting like this is so frustrating to read... I'm assuming there's no way to split this across multiple lines with JSDoc right? Is that another limitation we could add to the list of reasons why full TS is preferred?
💯 It's very unpleasant. I don't believe TypeScript + JSDoc work properly when we break these on multiple lines. It would be much nicer in TypeScript. For reference, this is equivalent TypeScript:
export type AdditionalProps<T> =
T extends React.ComponentType<infer U>
? U
: T extends string
? React.ComponentPropsWithoutRef<'span'>
: {};Fortunately, although it's exported (JSDoc typedefs are always exported, but this likely isn't easily accessible from the package) it's really an internal type helper.
Co-authored-by: Sara Marcondes <[email protected]>
|
Hi @sirreal, I have been trying to find out if this PR is still valid today. In the latest trunk, the And thank you for your contribution! |
|
This was fine to close, thanks for the cleanup 👍 |
Description
Add types to the Icon component.
How has this been tested?
Types are properly generated (no test failures).
Types of changes
Internal: Code Quality.