Skip to content

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Jun 4, 2025

Previous Behavior

New Behavior

Related Issue(s)

@Hotell Hotell changed the title React 18/ref attr usage fix(react-components): use custom RefAttributes instead of React.RefAttributes Jun 4, 2025
Copy link

github-actions bot commented Jun 4, 2025

📊 Bundle size report

✅ No changes found

Copy link

github-actions bot commented Jun 4, 2025

Pull request demo site: URL

@Hotell Hotell force-pushed the react-18/ref-attr-usage branch from 5e71b4c to 9462a63 Compare June 4, 2025 13:39
@@ -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<
Copy link
Contributor Author

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>>;
Copy link
Contributor Author

@Hotell Hotell Jun 4, 2025

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

@Hotell Hotell force-pushed the react-18/ref-attr-usage branch from 34c4a8b to 9f80d96 Compare June 6, 2025 12:23
*/
export const getIntrinsicElementProps = <
Props extends UnknownSlotProps,
ExcludedPropKeys extends Extract<keyof Props, string> = never,
>(
/** The slot's default element type (e.g. 'div') */
Copy link
Contributor Author

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',
Copy link
Contributor Author

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",
Copy link
Contributor Author

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

@Hotell Hotell marked this pull request as ready for review June 6, 2025 13:09
@Hotell Hotell requested review from mltejera, a team and GeoffCoxMSFT as code owners June 6, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants