fluentui
fluentui copied to clipboard
[Bug]: slot API is broken on react v18
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.
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:
- 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 onImagePropsfor example https://github.com/microsoft/fluentui/issues/29578) - 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:
- a resolved slot component type (
slot.optional(props.slotName, {elementType: 'div'})this is an example of a resolved slot component type.props.slotNamethis 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. - name clashes between slot records and native properties https://github.com/microsoft/fluentui/issues/28693#issuecomment-1659737176 https://github.com/microsoft/fluentui/issues/29596
There are 3 main problems right now on the slot signature:
childrenshould not be a well definedReactNodeas this is not true in every slot (there are void slots) (at the moment slots signature it's a well definedchildren?: React.ReactNode, this is the main reason for the fail onImagePropsfor example https://github.com/microsoft/fluentui/issues/29578)childrenshould support a render function on the external signature, but it should remove it from the internal one (as soon as we stop defining children asReactNodewe'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 isReactNode)- 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
OverlayDrawerusesDialoginternally as a slot, butDialogchildren property does not follow slot signature) https://github.com/microsoft/fluentui/blob/4c36df0e2a7bf9d54d5137fabe74c720bdddebbc/packages/react-components/react-drawer/src/components/DrawerOverlay/useDrawerOverlay.ts#L49-L66
We could consider as an extra problem but not directly related to the slot signature:
- name clashing of
@types/reactHTML elements attributes and declared slots such ascontenthttps://github.com/microsoft/fluentui/issues/29596
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?
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
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.
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
Thanks for the investigations, this will help mapping the problem even further. Here's another issue linked to this one also https://github.com/microsoft/fluentui/issues/31482
Seems like this solution https://github.com/microsoft/fluentui/pull/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
This issue has not had activity for over 180 days! We're adding Soft close label and will close it soon for house-keeping purposes. Still require assistance? Please add comment - "keep open".
keep open
The same issue. Have to set "skipLibCheck": true, in tsconfig.json.
This issue should be resolved by #34154. Reassigning to @dmytrokirpa
The only workaround I found currently is:
<Sidenav
value="Sidenav"
item={{
children: ((_, props) => (
<div {...props}>{props.value}</div>
)) as SlotRenderFunction<T> as any,
}}
/>