[Bug]: slot API is broken on react v18

[bsunderhus]

Library

React Components / v9 (@fluentui/react-components)

Bug Description

Actual Behavior

Currently the slot API types are not compatible with react v18 types provided by @types/react, many components are breaking due to incompatibility with types that weren't detect previously.

For example, the ImageSlot types does not satisfies SlotPropsRecord which is a requirement for ImageProps:

In this case the main reason is the assumption of the requirement of a children attribute which is well defined as React.ReactNode, meanwhile ImageProps actually required children to be never

Expected Behavior

No errors on v18 for the slot API.

[bsunderhus]

So basically while investigating our slot API to ensure compatibility with React v18, it turns out that our major flaws in the types revolves around the children property, some of those problems are:

1. if there's a children property or not (at the moment it's a well defined children?: React.ReactNode, this is the main reason for the fail on ImageProps for example [Bug]: slot API is broken on react v18 #29578)
2. if this children property includes a render function or not

our UnknownSlotProps is the default signature of what a slot should be, and it declares children as an optional property (with a value of React.ReactNode, which is not valid in some cases). If we want our components to be compliant as a slot then they should have that property to follow up on the UnknownSlotProps signature.

Here's also another problems that we fail to address at the moment:

1. a resolved slot component type (slot.optional(props.slotName, {elementType: 'div'}) this is an example of a resolved slot component type. props.slotName this is an example of an unresolved sot, or a slot shorthand) should remove from its children signature the render function, as once a slot is resolved it should move the render function from children to an internal symbol.
2. name clashes between slot records and native properties [Bug]: TypeScript 5.0 build error when specifying content attribute of TreeItemLayout #28693 (comment) [Bug]: content property is name clashing between react types and some v9 components  #29596

[bsunderhus]

There are 3 main problems right now on the slot signature:

1. children should not be a well defined ReactNode as this is not true in every slot (there are void slots) (at the moment slots signature it's a well defined children?: React.ReactNode, this is the main reason for the fail on ImageProps for example [Bug]: slot API is broken on react v18 #29578)
2. children should support a render function on the external signature, but it should remove it from the internal one (as soon as we stop defining children as ReactNode we'll have the problem of leaking the render function signature, so this is a problem we're avoiding at the moment by erroneously stating that children is ReactNode)
3. A v9 component should probably be "slot compliant" (meaning, should a v9 component follow the slot signature to be used as a slot? E.g OverlayDrawer uses Dialog internally as a slot, but Dialog children property does not follow slot signature)
   

   fluentui/packages/react-components/react-drawer/src/components/DrawerOverlay/useDrawerOverlay.ts

   Lines 49 to 66 in 4c36df0

   	const dialog = slot.always(
   	{
   	open: true,
   	defaultOpen,
   	onOpenChange,
   	inertTrapFocus,
   	modalType,
   	/*
   	* children is not needed here because we construct the children in the render function,
   	* but it's required by DialogProps
   	*/
   	children: null as unknown as JSX.Element,
   	},
   	{
   	elementType: Dialog,
   	},
   	);

We could consider as an extra problem but not directly related to the slot signature:

1. name clashing of @types/react HTML elements attributes and declared slots such as content [Bug]: content property is name clashing between react types and some v9 components  #29596

[rgylesbedford]

I've run into this issue as well when using the slot API. Is there any guidance for temporary workarounds apart from downgrading to React 17?

[bsunderhus]

Sadly there's no workaround at the moment @rgylesbedford, we'll be providing a proper fix once the issues here highlighted have been properly discussed! I'll keep this issue posted on this topic

[microsoft-github-policy-service]

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.