-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
change/@fluentui-react-utilities-66a3d122-bdde-45be-a7b3-d6ae2b28a1d6.json
Show resolved
Hide resolved
5e71b4c to
9462a63
Compare
packages/react-components/react-utilities/src/compose/getIntrinsicElementProps.ts
Show resolved
Hide resolved
| ); | ||
| }); | ||
| // 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
|
|
||
| // @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
| 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/react@18.2.61 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
RefAttributesinterface which is used within ForwardRefComponent to mitigateReact.RefAttributesexpansion causing breaking changes in v8/v9 mixed context - shipped as patch release in @types/react@18.2.61 #34572React.RefAttributeuse in v9React.RefAttributewith customRefAttributein v9 codeRelated Issue(s)