-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(react-components): use custom RefAttributes instead of React.RefAttributes #34590
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
base: master
Are you sure you want to change the base?
Conversation
📊 Bundle size report✅ No changes found |
Pull request demo site: URL |
change/@fluentui-react-utilities-66a3d122-bdde-45be-a7b3-d6ae2b28a1d6.json
Outdated
Show resolved
Hide resolved
5e71b4c
to
9462a63
Compare
packages/react-components/react-utilities/src/compose/getIntrinsicElementProps.ts
Show resolved
Hide resolved
@@ -51,6 +51,11 @@ export const CheckboxShim = React.forwardRef((props: ICheckboxProps, _ref: React | |||
indicator={{ className: mergeClasses('ms-Checkbox-checkbox', styles.checkbox) }} | |||
/> | |||
); | |||
}); | |||
// NOTE: cast is necessary as `ICheckboxProps` extends React.Ref<HTMLDivElement> which is not compatible with our defined React.Ref<HTMLInputElement> | |||
}) as React.ForwardRefExoticComponent< |
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.
unified pattern with the rest of migration code - in this case simple advance declaration wouldn't match because the ref generic mismatch
@@ -42,7 +42,7 @@ export const brandWeb: BrandVariants; | |||
export const ButtonShim: React_2.ForwardRefExoticComponent<IBaseButtonProps & React_2.RefAttributes<HTMLButtonElement>>; | |||
|
|||
// @public (undocumented) | |||
export const CheckboxShim: React_2.ForwardRefExoticComponent<Pick<ICheckboxProps, "label" | "title" | "className" | "key" | "disabled" | "name" | "defaultChecked" | "id" | "onChange" | "componentRef" | "styles" | "theme" | "checked" | "ariaLabel" | "required" | "ariaDescribedBy" | "ariaLabelledBy" | "ariaPositionInSet" | "ariaSetSize" | "boxSide" | "checkmarkIconProps" | "defaultIndeterminate" | "indeterminate" | "inputProps" | "onRenderLabel"> & React_2.RefAttributes<HTMLInputElement>>; | |||
export const CheckboxShim: React_2.ForwardRefExoticComponent<ICheckboxProps & React_2.RefAttributes<HTMLInputElement>>; |
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.
this is the same API without unwrapping -> result of normalizing patterns and using explicit function types
34c4a8b
to
9f80d96
Compare
*/ | ||
export const getIntrinsicElementProps = < | ||
Props extends UnknownSlotProps, | ||
ExcludedPropKeys extends Extract<keyof Props, string> = never, | ||
>( | ||
/** The slot's default element type (e.g. 'div') */ |
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.
this is invalid "JSDOC" - moved to proper JSDoc format
types: { | ||
'React.RefAttributes': { | ||
message: | ||
'`React.RefAttributes` is leaking string starting @types/[email protected] creating invalid type contracts. Use `RefAttributes` from @fluentui/react-utilities instead', |
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.
tbh this is not the most robust rule as it doesn't work on interface extensions, and also doesn't distinguishes about the imports. the autofixer is also naive as it won't provide proper import.
for now it's good enough, but in future we will introduce custom rule
@@ -0,0 +1,7 @@ | |||
{ | |||
"type": "none", |
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.
although api.md changed the type is exactly the same -> thus no bump
Previous Behavior
New Behavior
RefAttributes
interface which is used within ForwardRefComponent to mitigateReact.RefAttributes
expansion causing breaking changes in v8/v9 mixed context - shipped as patch release in @types/[email protected] #34572React.RefAttribute
use in v9React.RefAttribute
with customRefAttribute
in v9 codeRelated Issue(s)