-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[core] Offer alternative to OverridableComponent
via module augmentation for better performance
#32735
[core] Offer alternative to OverridableComponent
via module augmentation for better performance
#32735
Changes from 19 commits
5007238
0752787
7ecd73c
8870553
3870e8e
28e5b51
5d4f616
cace4e4
278c3fe
4123794
7f7da10
769ab83
08707d9
51cb893
4eace55
0059bf4
96f9ecd
d23b5d4
c5761e0
97b0ed9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import * as React from 'react'; | ||
import { DistributiveOmit } from '@mui/types'; | ||
|
||
declare module '@mui/material/OverridableComponent' { | ||
/** | ||
* A component whose root component can be controlled via a `component` prop. | ||
* | ||
* Adjusts valid props based on the type of `component`. | ||
*/ | ||
interface OverridableComponent<M extends OverridableTypeMap> { | ||
<C extends React.ElementType>( | ||
props: { | ||
/** | ||
* The component used for the root node. | ||
* Either a string to use a HTML element or a component. | ||
*/ | ||
component: C; | ||
} & OverridePropsVer2<M, C>, | ||
): JSX.Element; | ||
(props: DefaultComponentPropsVer2<M>): JSX.Element; | ||
} | ||
|
||
/** | ||
* Props of the component if `component={Component}` is used. | ||
*/ | ||
// prettier-ignore | ||
type OverridePropsVer2< | ||
M extends OverridableTypeMap, | ||
C extends React.ElementType, | ||
> = ( | ||
& BaseProps<M> | ||
& DistributiveOmit<React.ComponentPropsWithoutRef<C>, keyof BaseProps<M>> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think the usage of The reason I ask is that the React.ComponentPropsWithoutRef has a lot of keys and omitting some of the keys sounds like it requires expensive computation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we can get away without There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Nope, it was one of the first things I tried replacing, this is why I left it. |
||
& { ref?: React.Ref<any> } | ||
mnajdova marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
|
||
/** | ||
* Props if `component={Component}` is NOT used. | ||
*/ | ||
// prettier-ignore | ||
type DefaultComponentPropsVer2<M extends OverridableTypeMap> = | ||
& BaseProps<M> | ||
& DistributiveOmit<React.ComponentPropsWithoutRef<M['defaultComponent']>, keyof BaseProps<M>> | ||
& { ref?: React.Ref<any> }; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
{ | ||
"extends": "../../tsconfig", | ||
"include": ["src/**/*", "test/**/*"], | ||
"exclude": ["test/typescript/moduleAugmentation"] | ||
"exclude": ["test/typescript/moduleAugmentation", "src/OverridableComponentAugmentation.ts"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import * as React from 'react'; | ||
|
||
declare module '@mui/types' { | ||
/** | ||
* A component whose root component can be controlled via a `component` prop. | ||
* | ||
* Adjusts valid props based on the type of `component`. | ||
*/ | ||
interface OverridableComponent<M extends OverridableTypeMap> { | ||
<C extends React.ElementType>( | ||
props: { | ||
/** | ||
* The component used for the root node. | ||
* Either a string to use a HTML element or a component. | ||
*/ | ||
component: C; | ||
} & OverridePropsVer2<M, C>, | ||
): JSX.Element; | ||
(props: DefaultComponentPropsVer2<M>): JSX.Element; | ||
propTypes?: any; | ||
} | ||
|
||
/** | ||
* Props of the component if `component={Component}` is used. | ||
*/ | ||
// prettier-ignore | ||
type OverridePropsVer2< | ||
M extends OverridableTypeMap, | ||
C extends React.ElementType | ||
> = ( | ||
& BaseProps<M> | ||
& DistributiveOmit<React.ComponentPropsWithoutRef<C>, keyof BaseProps<M>> | ||
& { ref?: React.Ref<any> } | ||
); | ||
|
||
/** | ||
* Props if `component={Component}` is NOT used. | ||
*/ | ||
// prettier-ignore | ||
type DefaultComponentPropsVer2<M extends OverridableTypeMap> = | ||
& BaseProps<M> | ||
& DistributiveOmit<React.ComponentPropsWithoutRef<M['defaultComponent']>, keyof BaseProps<M>> | ||
& { ref?: React.Ref<any> }; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
{ | ||
"extends": "../../tsconfig" | ||
"extends": "../../tsconfig", | ||
"exclude": ["OverridableComponentAugmentation.ts"] | ||
} |
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.
If we're going to use these types across the codebase at some point, it would be great to rename the generic props to something more meaningful.
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 am keeping it as it was before, so that it is visible what exactly are the changes between the two, but yes, we can rename the generics at any point in time :)
Can be done in a dedicated PR where both the existing and the new types will be renamed.