Skip to content
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

Support new root handling API introduced in React 18.x #37

Open
eithe opened this issue Aug 22, 2022 · 10 comments
Open

Support new root handling API introduced in React 18.x #37

eithe opened this issue Aug 22, 2022 · 10 comments

Comments

@eithe
Copy link

eithe commented Aug 22, 2022

@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.

@Retsam
Copy link
Owner

Retsam commented Aug 23, 2022

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.

@eithe
Copy link
Author

eithe commented Apr 20, 2023

Had a chance yet, @Retsam?

@eithe
Copy link
Author

eithe commented Jun 22, 2023

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
Copy link
Author

eithe commented Feb 5, 2024

Any news @Retsam ?

@Retsam
Copy link
Owner

Retsam commented Feb 12, 2024

@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)

@eithe
Copy link
Author

eithe commented Mar 5, 2024

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.

@Retsam
Copy link
Owner

Retsam commented Mar 24, 2024

@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.

@eithe
Copy link
Author

eithe commented Mar 24, 2024

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

@Retsam
Copy link
Owner

Retsam commented Mar 24, 2024

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

master...react-18

@eithe
Copy link
Author

eithe commented Oct 23, 2024

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();
  });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants