-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
fix(react): update @types/react to v19+ #30085
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
The issue that needs to be fixed:
|
This PR seems to be missing adjustments in at least IonRouter, IonTab, and IonIcon. I tried it myself, but simply adding children and adjusting some refs wasn't enough. |
It would be helpful if you could post the specific errors you encountered. Also, hopefully we can get approval from someone at Ionic soon so that the workflows can be run, which should surface some more errors. |
I was able to successfully build Errors and warnings```sh npm run build
src/index.ts → dist/... 27 <PageManager (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type 'PropsWithoutRef & { forwardedRef: ForwardedRef; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes & Readonly'. 125 return <Overlay {...props} forwardedRef={ref} />; src/components/react-component-lib/createOverlayComponent.tsx: (140:13) 140 return <Overlay {...props} forwardedRef={ref} />; (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type 'RefObject<HTMLIonOverlayElement | null>' is not assignable to type 'RefObject'. 47 this.ref = React.createRef(); (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type 'RefObject<HTMLElement | null>' is not assignable to type 'RefObject'. 53 this.wrapperRef = React.createRef(); (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type 'RefObject<HTMLElement | null>' is not assignable to type 'RefObject'. 42 this.ref = React.createRef(); (!) Plugin typescript: @rollup/plugin-typescript TS2339: Property 'children' does not exist on type 'IonNavProps'. 21 const IonNavInternal: React.FC = ({ children, forwardedRef, ...restOfProps }) => { (!) Plugin typescript: @rollup/plugin-typescript TS18046: 'child.props' is of type 'unknown'. 95 (child.type === Fragment && child.props.children[0].type === IonRouterOutlet); (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type '{ className?: string | undefined; children: ReactNode | ChildFunction; onIonTabsDidChange?: ((event: IonTabsCustomEvent<{ tab: string; }>) => void) | undefined; onIonTabsWillChange?: ((event: IonTabsCustomEvent<...>) => void) | undefined; }' is not assignable to type 'Omit<HTMLAttributes, "style">'. 177 return <IonTabsInner {...this.props}>; (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type '{ children: Element; className: string; routeInfo: RouteInfo | undefined; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes & Readonly'. 195 <PageManager className={className ? (!) Plugin typescript: @rollup/plugin-typescript TS2345: Argument of type '{ (props: StencilReactExternalProps<PropType, ElementType>, ref: StencilReactForwardedRef): React.JSX.Element; displayName: string; }' is not assignable to parameter of type 'ForwardRefRenderFunction<ElementType, PropsWithoutRef<StencilReactExternalProps<PropType, ElementType>>>'. 40 return React.forwardRef(forwardRef); (!) Plugin typescript: @rollup/plugin-typescript TS2345: Argument of type '{ (props: IonicReactExternalProps<PropType, ElementType>, ref: React.ForwardedRef): React.JSX.Element; displayName: string; }' is not assignable to parameter of type 'ForwardRefRenderFunction<ElementType, PropsWithoutRef<IonicReactExternalProps<PropType, ElementType>>>'. 28 return React.forwardRef(forwardRef); (!) Plugin typescript: @rollup/plugin-typescript TS2554: Expected 1 arguments, but got 0. 18 const overlayRef = useRef(); node_modules/@types/react/index.d.ts:1722:24 src/hooks/useOverlay.ts: (25:22) 25 const overlayRef = useRef(); node_modules/@types/react/index.d.ts:1722:24 src/hooks/useOverlay.ts: (26:26) 26 const containerElRef = useRef(); node_modules/@types/react/index.d.ts:1722:24 src/lifecycle/hooks.ts: (8:14) 8 const id = useRef<number | undefined>(); node_modules/@types/react/index.d.ts:1722:24 src/lifecycle/hooks.ts: (21:14) 21 const id = useRef<number | undefined>(); node_modules/@types/react/index.d.ts:1722:24 src/lifecycle/hooks.ts: (34:14) 34 const id = useRef<number | undefined>(); node_modules/@types/react/index.d.ts:1722:24 src/lifecycle/hooks.ts: (47:14) 47 const id = useRef<number | undefined>(); node_modules/@types/react/index.d.ts:1722:24 (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type 'undefined' is not assignable to type 'OverlayType'. 38 overlayRef.current = undefined; src/hooks/useController.ts: (59:5) 59 overlayRef.current = undefined; src/hooks/useOverlay.ts: (79:7) 79 overlayRef.current = undefined; src/hooks/useOverlay.ts: (88:5) 88 overlayRef.current = undefined; (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type 'undefined' is not assignable to type 'HTMLDivElement'. 80 containerElRef.current = undefined; src/hooks/useOverlay.ts: (89:5) 89 containerElRef.current = undefined; (!) Plugin typescript: @rollup/plugin-typescript TS2339: Property 'children' does not exist on type 'Readonly'. 134 {this.props.children} (!) Plugin typescript: @rollup/plugin-typescript TS2322: Type 'RefObject<HTMLDivElement | null>' is not assignable to type 'RefObject'. 23 this.ionPageElementRef = React.createRef(); (!) Plugin typescript: @rollup/plugin-typescript TS2339: Property 'children' does not exist on type 'Readonly'. 85 const { className, children, routeInfo, forwardedRef, ...props } = this.props; (!) Plugin typescript: @rollup/plugin-typescript TS2339: Property 'children' does not exist on type 'Readonly'. 51 {show && this.props.children} created dist/ in 3.1s
|
@jadejr Thanks, that's helpful. A challenge is that on HEAD, there are a lot of warnings even before the MR:
|
I pushed a few more fixes for missing children props, including some that were already broken on HEAD. I think this is ready for review despite all the warnings, because the warnings are already there in HEAD (as far as I can tell; the list is quite long so it was hard to compare). Long-term, I think the best fix would be to get rid of these wrappers as I proposed in #30107 |
@ptmkenny : Using the custom elements directly would be for the best, but how soon can ionic actually do that? Seems like it would require a semver major bump since the packages are versioned collectively and I doubt they'd wanna do that just for react. |
It was on me for not realizing those warnings weren't actually a problem, but here is an actual issue in my code that uses Same error regarding args for
|
My understanding of web components is limited, but I think that support can be added without replacing the React components. In other words, the React components would be kept available (for use with versions of React before 19, which will likely be supported for years anyway), but the web components would also be made available for direct use with React 19. If you have more comments on this, please add them to the other issue. |
Issue number: resolves #29991
What is the current behavior?
React types are currently being tested against React 17, but React 19 is the current version.
What is the new behavior?
Does this introduce a breaking change?
I don't think this introduces a breaking change but this needs confirmation because of the type changes.
Also, the dev dependency is react 17, which makes it tricky to test with 19.
Other information
I don't know if this is ready to go yet; I made this PR to run the tests.