-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(react-components): add react 18 support #34456
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
base: master
Are you sure you want to change the base?
Conversation
📊 Bundle size reportUnchanged fixtures
|
Pull request demo site: URL |
change/@fluentui-react-accordion-70e37d4f-804b-494a-9d94-31840668c018.json
Outdated
Show resolved
Hide resolved
...s/react-drawer/library/src/components/DrawerHeaderTitle/useDrawerHeaderTitleStyles.styles.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-menu/library/src/components/MenuItem/useMenuItem.tsx
Show resolved
Hide resolved
...ages/react-components/react-portal/library/src/components/Portal/usePortalMountNode.test.tsx
Outdated
Show resolved
Hide resolved
...ct-components/react-tag-picker/library/src/components/TagPickerButton/useTagPickerButton.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/compose/deprecated/getSlots.ts
Show resolved
Hide resolved
packages/react-components/react-breadcrumb/stories/src/Breadcrumb/index.stories.tsx
Outdated
Show resolved
Hide resolved
Lets go! Woohoo!! 🥳 |
80ea1e6
to
0fb509e
Compare
0fb509e
to
bc4bf6d
Compare
bc4bf6d
to
2af8cd2
Compare
@@ -72,6 +72,7 @@ import type { | |||
...defaultMenuProps, | |||
}; | |||
|
|||
// @ts-expect-error - Slot type mismatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: add proper explanation
@@ -10,14 +10,16 @@ import { useCustomStyleHook_unstable } from '@fluentui/react-shared-contexts'; | |||
* DataGridBody component | |||
*/ | |||
export const DataGridBody: ForwardRefComponent<DataGridBodyProps> & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we need the type declaration part since we already do type-casting.
2af8cd2
to
98cb88c
Compare
@@ -45,7 +45,9 @@ jobs: | |||
run: yarn nx affected -t bundle-size --nxBail | |||
|
|||
- name: Compare bundle size with base | |||
run: npx monosize compare-reports --branch=${{ github.event.pull_request.base.ref }} --output=markdown --quiet > ./monosize-report.md | |||
# FIXME: undo this before PR to master | |||
run: npx monosize compare-reports --branch=master --output=markdown --quiet > ./monosize-report.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: revert before merge
@@ -74,6 +74,7 @@ import type { | |||
|
|||
slot.always(props.menuButton, { | |||
defaultProps: menuButtonDefaultProps, | |||
// @ts-expect-error - Slot type mismatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: add a proper explanation
7099472
to
2074317
Compare
Summary
This PR addresses type incompatibilities introduced by React 18 that break our Slots API. Specifically, it fixes issues with
ReactNode
,RefAttributes
, and several slot-related utility types. It also adjusts internal type declarations to better align with actual usage and React’s updated typings.Previous Behavior
1.
ReactNode
Breaking ChangeReact 18 introduced stricter definitions for
ReactNode
, removing the{}
catch-all inReactFragment
. This change breaks our slot typings, which previously relied on the broader compatibility of React 17:React 17:
React 18:
🔗 Playground showing the issue
2.
RefAttributes
ChangeIn React 18,
RefAttributes
now usesLegacyRef<T>
which includes support for string refs — something we don't support in our API.React 17:
React 18:
3.
FC
doesn't provides type forchildren
prop, it should be explicitly definedReact 17:
React 18:
New Behavior
Loosened type constraints in
UnknownSlotProps
Previously, the
children
prop has typeReactNode | undefined
, but this doesn’t work with theSlotRenderFunction
and React 18. This change removes that restriction and to enable usage with React 18.Updated
SlotShorthandValue
to acceptIterable<ReactNode>
React 18 expands
ReactNode
to includeIterable<ReactNode>
, so this change ensures our shorthand slot values also support this structure. This enables support for JSX fragments and iterable rendering.WithSlotRenderFunction
adjusted for manual type assertions in React 18In React 18, implicit inference of slot render functions may not work correctly due to stricter type definitions. Consumers must now use
as
orsatisfies
type assertions when passing a function to slot props.Removed deprecated
VoidFunctionComponent
VoidFunctionComponent
is deprecated and unnecessary sinceComponentType
handles the same use cases. Removing it simplifies our generic types and avoids confusion.cloneTriggerTree
updated for broaderchildren
supportThis utility function now assumes
children
can be more than justReactNode
. It aligns with the loosened constraints and supports broader use cases, including render functions or iterables.Removed usage of
React.RefAttributes
in several utilitiesWith React 18,
RefAttributes
includesLegacyRef
, which supports string refs — something we don’t allow. We’ve replaced it with more precise types to enforce our internal API constraints.Added
{ children?: React.ReactNode }
to fix story/test compatibilitySome stories and test utilities failed under React 18 due to missing
children
prop typing. So the fix is too use theReact.PropsWithChildren<Props>
or explicitly declare thechildren
prop type.React 18 type changes:
Slot render functions must now be manually typed in React 18:
Future Plans
Migrate slot render function away from
children
to a dedicated prop to improve type inference.Related Issues