[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.

[bworline]

While investigating this I created a repro repo that may help.

In the provided repo, open Test.ts and Test2.ts. After initialization you will see red squiggles saying, "Type 'x' does not satisfy the constraint 'SlotPropsRecord'."

Note that this repo has a tsconfig "lib" value of es2020. If changed to "es5" the type error goes away. However, look at the difference in how TypeScript is seeing the types in Test2.ts when hovering over menuListWapper: 'any' is hiding the issue. I've found that a lib value of es2015 or greater (or even just es2015.iterable) will cause the issue to manifest.

lib: es5

lib: es2020

[bsunderhus]

Thanks for the investigations, this will help mapping the problem even further. Here's another issue linked to this one also #31482

[bsunderhus]

Seems like this solution #29865 isn't enough for React 18. It's still breaking due to not properly removing the content property from the slot, but from the props only