React 18 support #148
React 18 support #148
Conversation
|
|
||
| // required - one or the other | ||
| rootComponent: null, | ||
| loadRootComponent: null, | ||
|
|
||
| // optional opts | ||
| renderType: null, | ||
| renderType: "createRoot", |
There was a problem hiding this comment.
Should this be considered a breaking change?
There was a problem hiding this comment.
Hmm, yeah, probably, since it changes a lot of how React itself renders things.
| export interface SingleSpaReactOpts<RootComponentProps> { | ||
| React: typeof React; | ||
| ReactDOM: typeof ReactDOM; | ||
| React: any; |
There was a problem hiding this comment.
Whenever possible, we should avoid the any type. I'm not sure what was wrong with the previously used type typeof React since React is a peer dependency. If I'm understanding peer dependencies correctly (and I'm not really sure if I am), the type of that import would be defined by the version of React that the user has.
And the same point for the other props.
So I would expect the types to be:
import * as ReactDOM from "react-dom";
import * as ReactDOMClient from "react-dom/client";
//...
export interface SingleSpaReactOpts<RootComponentProps> {
React: typeof React;
ReactDOM?: typeof ReactDOM;
ReactDOMClient?: typeof ReactDOMClient;
//...
}Maybe the source of the problem in @hijiangtao's comment #142 (comment) is that react-dom is not a peer dependency? Can it be @filoxo? Or maybe the source of the issue is just that my grasp on types from peer-dependencies is just off. Could be! In which case, forgive me for this PR comment of mine.
There was a problem hiding this comment.
@dgreene1 right, please also see #144 (review) and #144 (comment). if I'm using React 17 still and my version of react types doesn't have ReactDOMClient, wouldn't this completely break? The whole point is that we want backwards compatibility so using ReactDOMClient type is not possible without making single-spa-react incompatible with react 17 and below. At least that's my understanding.
|
Has React 18 support made it to npm? version 5.1.1 still has the old code in minified version.
|
Resolves #142 and #115.