Skip to content
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

[Bug]: content property is name clashing between react types and some v9 components #29596

Closed
bsunderhus opened this issue Oct 19, 2023 · 5 comments · Fixed by #29865
Closed

Comments

@bsunderhus
Copy link
Contributor

bsunderhus commented Oct 19, 2023

Library

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

Bug Description

Seems like @types/react defines that content is a valid property that accepts string | undefined as it's type on HTMLAttributes interface:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/v17/index.d.ts#L1876

content?: string | undefined;

This interface is one of the building blocks of every native element property declaration, which mean that every single one of v9 component that have a well defined root as a native element, have this property declared by HTMLAttributes interface. In these components:

  1. AvatarGroupPopover
  2. TableCellLayout
  3. Tab
  4. Tooltip (in tooltip it's not a problem, as there's no clash happening)
  5. MenuItem
  6. ToastContainer

This will be a problem, as content is also a sot declaration, which will cause a clash between native element types and the slot definition, making the slot virtually equivalent to string | undefined, here's an example with MenuItem https://codesandbox.io/s/loving-hill-9t3t5z?file=/example.tsx

@bsunderhus
Copy link
Contributor Author

Here's another case where this happened in -preview Tree #28693 (comment)

@layershifter
Copy link
Member

See #27425.

@bsunderhus
Copy link
Contributor Author

bsunderhus commented Oct 23, 2023

I guess the only solution for this that would not involve a breaking change would be to deprecate every single slot that causes this name clash and rename it to something like main (which is the solution adopted by #28693 (comment)), WDYT @Hotell @layershifter ?

@bsunderhus
Copy link
Contributor Author

If we're considering some breaking change then we can maybe discuss the modification of the root type to omit RDFa types from @types/react base types.

@bsunderhus
Copy link
Contributor Author

bsunderhus commented Nov 16, 2023

Seems like on some specific versions of @types/react@17 this problem is solved, from v17.0.44 to v17.0.55 the content property is removed from HTMLAttributes https://codesandbox.io/s/musing-currying-fyvtjx?file=/example.tsx

Seems like this RDFa props have been added at 17.0.56 DefinitelyTyped/DefinitelyTyped#64972

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment