baseweb icon indicating copy to clipboard operation
baseweb copied to clipboard

Support for React 18

Open simPod opened this issue 2 years ago • 8 comments

Hello, are there plans to support React 18?

E.g. currently RefForwardingComponent is used which does not exists in React 18.

simPod avatar Apr 07 '22 20:04 simPod

We just started testing against react 18 https://github.com/uber/baseweb/pull/4867. If I understand correctly, RefForwardingComponent is typescript related? We don't use ts at uber right now, so if you have suggestions to improve, thats welcome

chasestarr avatar Apr 07 '22 21:04 chasestarr

For what it's worth, I'm running into this as well when using typescript with react 18 and base web (on a fresh install), the error looks like this:

webpack 5.72.0 compiled with 1 error in 71 ms
ERROR in ./node_modules/baseui/overrides.ts 11:11-33
TS2694: Namespace 'React' has no exported member 'RefForwardingComponent'.
     9 | type ComponentOverride<T> =
    10 |   | React.ComponentType<T>
  > 11 |   | React.RefForwardingComponent<T>;
       |           ^^^^^^^^^^^^^^^^^^^^^^
    12 |
    13 | interface OverrideObject<T> {
    14 |   component?: ComponentOverride<T>;

A quick google search mentions something about this type over here: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/46691 but I'm not deep enough into Typescript to understand what is going on and how to fix it.

advdv avatar Apr 12 '22 12:04 advdv

@advanderveer I've fixed it in the PR above ur post.

simPod avatar Apr 12 '22 13:04 simPod

@simPod thank you, I thought that pr was a longshot because it was dependant on dropping react 16 support, but I see now that this has just been ok'ed. So, nice!

btw: For everyone that is really impassion, I've temporarily "solved" it by reverting JUST the types package back to 17:

"@types/react": "^17.0.0",

that at least makes it compile for now, might cause various other issues of course

advdv avatar Apr 12 '22 19:04 advdv

@advanderveer this is the solution I use until the PR is released:

react.d.ts:

declare module 'react' {
  // eslint-disable-next-line max-len
  export type RefForwardingComponent<T, P extends Record<string, unknown> = Record<string, unknown>> = ForwardRefRenderFunction<T, P>;
}

simPod avatar Apr 12 '22 20:04 simPod

react.d.ts:

declare module 'react' {
  // eslint-disable-next-line max-len
  export type RefForwardingComponent<T, P extends Record<string, unknown> = Record<string, unknown>> = ForwardRefRenderFunction<T, P>;
}

@simPod Thank you for the solution, but I'm not 100% sure how to get the type system to see this as an extension of the existing types defined for react?. It seems to be ignoring the existing types as per below:

> tsc && vite build                                                                                                                                                                                       
                                                                                                                                                                                                          
node_modules/baseui/overrides.ts:7:38 - error TS2694: Namespace '"react"' has no exported member 'PropsWithChildren'.                                                                                     
                                                                                                                                                                                                          
7   | ((props: {$theme: Theme} & React.PropsWithChildren<T>) => StyleObject);                                                                                                                             
                                       ~~~~~~~~~~~~~~~~~                                                                                                                                                  
                                                                                                                                      

jacquesg avatar Apr 23 '22 11:04 jacquesg

@chasestarr Hi there, there is another issue with TS & React 18. This one is regarding HeaderNavigation.

This:

export interface HeaderNavigationProps {
  overrides?: HeaderNavigationOverrides;
}

Should be this:

export interface HeaderNavigationProps {
  overrides?: HeaderNavigationOverrides;
  children?: React.ReactNode;
}

Otherwise normal usage of HeaderNavigation will produce error.

mbrain-io avatar May 19 '22 12:05 mbrain-io

What I've done while waiting for official support is to use patch-package to fix the issue I was having.

npm install patch-package

In package.json I have

  "scripts": {
    "postinstall": "patch-package",
  },

Finally I have this patch (patches/baseui+11.0.3.patch) which was generated based on local changes I had made to node_modules:

diff --git a/node_modules/baseui/overrides.ts b/node_modules/baseui/overrides.ts
index 9d57041..0b22c30 100644
--- a/node_modules/baseui/overrides.ts
+++ b/node_modules/baseui/overrides.ts
@@ -8,7 +8,7 @@ type StyleOverride<T> =
 
 type ComponentOverride<T> =
   | React.ComponentType<T>
-  | React.RefForwardingComponent<T>;
+  | React.ForwardRefRenderFunction<T>;
 
 interface OverrideObject<T> {
   component?: ComponentOverride<T>;

jacquesg avatar Jun 07 '22 12:06 jacquesg

forceUpdate() in LayersManager (check here) giving following error after migrating to React18

Reproduced the same issue in This CodeSandbox

Screenshot 2022-09-29 at 2 06 47 PM

HemangNakarani avatar Sep 30 '22 06:09 HemangNakarani

HeaderNavigation types still don't allow children. PR to fix this issue #5180. In the mean time it could be fixed via

const HeaderNavigationFix = (props: React.PropsWithChildren<HeaderNavigationProps>) => <HeaderNavigation {...props} />;

alexgorbatchev avatar Sep 30 '22 22:09 alexgorbatchev