ko-react icon indicating copy to clipboard operation
ko-react copied to clipboard

Support new root handling API introduced in React 18.x

Open eithe opened this issue 3 years ago • 11 comments

@Retsam Have you looked into supporting React 18.x with regards to the new root API?

Specifically it affects the React binding handler and ReactDOM.unmountComponentAtNode + ReactDOM.render.

eithe avatar Aug 22 '22 12:08 eithe

Yeah, I'd like to add support for it. I'm going to have to investigate whether I can add support for the new API (for React 18) while still supporting the old API (for React < 18), or if it'll just have to be a breaking change and drop support for previous React runtimes.

Retsam avatar Aug 23 '22 13:08 Retsam

Had a chance yet, @Retsam?

eithe avatar Apr 20 '23 06:04 eithe

I had a quick go, seems to run, but had to leave it there for now so should be considered alpha and untested.

interface KnockoutReactHTMLElement extends HTMLElement {
    _reactRoot?: typeof ReactDOM.Root;
}

export const ReactBindingHandler = (): KnockoutBindingHandler<KnockoutReactHTMLElement, ReactBindingValue> => ({
    init(element) {
        ko.utils.domNodeDisposal.addDisposeCallback(element, () => {
            const root = element._reactRoot;
            if (root) {
                root.unmount();
            }
        });
        return { controlsDescendantBindings: true };
    },
    update(element, valueAccessor) {
        const { Props, Component } = ko.unwrap(valueAccessor());
        if (!Component) {
            throw new Error("No React component provided");
        }

        const unwrappedProps = ko.unwrap(Props);
        // Ignore dependencies to avoid unwanted subscriptions to observables
        // inside render functions
        ko.ignoreDependencies(() => {
            const root = element._reactRoot;
            if (root) {
                root.render(React.createElement(Component, unwrappedProps));
            } else {
                element._reactRoot = createReactRoot(element);
                element._reactRoot.render(React.createElement(Component, unwrappedProps));
            }
        });
    },
});

const createReactRoot = (element: HTMLElement) => {
    const root = ReactDOM.createRoot(element);
    return {
        render: (component: React.ReactElement) => {
            root.render(component);
        },
        unmount: () => {
            root.unmount();
        },
    };
}

eithe avatar Jun 22 '23 08:06 eithe

Any news @Retsam ?

eithe avatar Feb 05 '24 09:02 eithe

@eithe Thanks for the reminder; I'm working on this now. It looks like supporting both is tricky, so it's going to be a full upgrade to React 18 for the repo with a breaking change.

I think I have the change itself done, but I'm also going to have to rip out enzyme (which doesn't support React 18) if I want to have any tests working, so that may take a little bit.

(Also my main codebase I use this in hasn't updated to React 18, so I'm working on that too)

Retsam avatar Feb 12 '24 18:02 Retsam

Any thoughts on my draft above, @Retsam? We are thinking about moving ahead with this now, but if you have any concerns I would like to hear them.

eithe avatar Mar 05 '24 10:03 eithe

@eithe Sorry for the delay, I've published a 1.0.0-beta-1 version that should support React 18, if you want to try it. I've done a bit of preliminary testing, but nothing extensive.

Retsam avatar Mar 24 '24 00:03 Retsam

Sounds good. Do you have a branch with those changes as well?

eithe avatar Mar 24 '24 16:03 eithe

@eithe Ah, yeah, forgot to actually push the branch - it's now up here

https://github.com/Retsam/ko-react/compare/master...react-18

Retsam avatar Mar 24 '24 20:03 Retsam

Unfortunately I haven't had the time to test out your code yet, but just FYI I had to add the following near the if (root) line in my code so that React could finish rendering properly before unmounting. We have had some scenarios where the could be a warning from React without this.

if (root?.unmount) {
  requestAnimationFrame(() => {
    root.unmount();
  });
}

eithe avatar Oct 23 '24 08:10 eithe