Begin rewrite to typescript and only React 19 compatibility

[internettrans]

No description provided.

[ibacher]

I like API here a lot. A few minor points on Typescript stuff, but it feels pretty elegant. I actually really like the idea of passing the functions in rather than the whole of react-dom/client.

I know this is early stage, but is it part of the goal of the rewrite to drop support for things like the SingleSpaProps context and SingleSpaRoot? I'm actually somewhat in favour of doing so (with OpenMRS, we already have a wrapper that can provide similar facilities, so this allows me to opt-out of stuff I don't need in the React tree).

[ibacher]

To help with client code, maybe this signature would work:

Suggested change

	createReactNode: ((props) => ReactNode) | ((props) => Promise<ReactNode>);
	createReactNode<T = any>(props: T): ReactNode | Promise<ReactNode>;

It should be basically equivalent with the addition that if there is a type for the props, the createReactNode() implementation can be fully typed.

[ibacher]

Slightly more complicated, but something like this:

type AtLeastOne<T> = { [Key in keyof T]: Pick<T, Key> }[keyof T];

export type SingleSpaReactOpts = DomElementGetterOpts & AtLeastOne<{
  createRoot: typeof createRoot;
  hydrateRoot: typeof hydrateRoot;
}> & {
  rootOptions?: RootOptions;
  createReactNode<T = any>(props: T): ReactNode | Promise<ReactNode>
}

Actually matches the requirements on line 27 and makes it possible (in TS) to pass in either createRoot() or hydrateRoot() or both.

[ibacher]

For the "both" case, it may be nice to have something like the old renderType property since that allows isomorphic app definitions, but it's probably not required.

[internettrans]

I know this is early stage, but is it part of the goal of the rewrite to drop support for things like the SingleSpaProps context and SingleSpaRoot?

I'm leaning against SingleSpaContext existing within single-spa-react, instead encouraging the following code in user-space:

export const SingleSpaContext = createContext();

const { init, mount, unmount } = singleSpaReact({
  createRoot,
  hydrateRoot,
  rootOptions:,
  createReactNode(props) {
    return (
      <SingleSpaContext value={props}>
        <App />
      </SingleSpaContext>
    );
  }
});

I'm also leaning against SingleSpaRoot. My understanding is that root.render() is synchronous and so there is no need to wait for componentDidMount() or useEffect() within the top-level component in order to know when to resolve the single-spa mount promise. It has been awhile since I've worked in this code - please let me know if you're aware of reasons why waiting for componentDidMount() is necessary to resolve the single-spa mount lifecycle. A wrapper around single-spa-react could offer the same functionality for those who wish for an upgrade path with less work. The SingleSpaRoot component has a display name which can be nice in react dev tools, but other than that I don't see much benefit to it.

[internettrans]

Update on SingleSpaRoot: the react docs indicate that root.render() is not synchronous the first time it is called. See https://react.dev/reference/react-dom/client/createRoot#root-render-caveats. So I think SingleSpaRoot should be added back to single-spa-react.

https://react.dev/reference/react-dom/client/createRoot#root-render-caveats

[ibacher]

Suggested change

	const configPromise = config();
	const configPromise = configProp();

[internettrans]

good find, updated and added a test

[ibacher]

Would it make sense to do something like const configPromise = Promise.resolve(configProp()); to allow both synchronous and asynchronous functions here?

[internettrans]

I don't see a reason why a synchronous function would be needed, since you could instead just provide configProp directly. There are no arguments passed to configProp when it's a function, so synchronous function has no use case from what I can tell

[ibacher]

It makes sense to remove the dom element assuming it was created during the mount, but at least in OpenMRS, we actually rely on pre-creating the dom nodes that single-spa apps render into. This is mostly to support things like a global layout (we have a global header, a global nav, and a global pop-in called "the workspace"; these things need to "wrap-around" arbitrary MFEs), so removing those domElements on unmount() could result in the app misbehaving when the app is remounted, since the dom element would now be in an arbitrary location.

[internettrans]

Good point, I believe that at Canopy they also are not removing the container elements. Also see this line within single-spa-layout, which removes the dom elements at the layout level rather than framework adapter level.

I think the best solution may be to remove the domElement only if it was created by dom-element-getter-helpers, like you suggested. I'll update the code for this.

[ibacher]

I'm leaning against SingleSpaContext existing within single-spa-react

Totally on-board with that.

So I think SingleSpaRoot should be added back to single-spa-react.

Makes sense since we do need to track when the node is actually rendered. That's too bad, but I don't think harmful per se.