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

Concerning React 18 and migration to createRoot #53

Open
wijnbladh opened this issue Jan 9, 2023 · 0 comments
Open

Concerning React 18 and migration to createRoot #53

wijnbladh opened this issue Jan 9, 2023 · 0 comments

Comments

@wijnbladh
Copy link

Helloes,

Here are same observations from my brief investigation on how this library could be updated to React 18 and most notably the createRoot mechanic. Hopefully these remarks can be useful if that journey ever comes around.

  1. Changing the render method invocation in inject method of ReactDomFactory.js to createRoot is quite straightforward, but we will probably need to stash the returned root values somewhere and use them later for unmounting in dispose

  2. Unit tests might break badly since in React 18 the rendering is no longer an instant synchronous operation and the unit tests seem to expect this, consider for example:

// tests/Bootstrapper.spec.js, it('should render a component', (done) => { ...
...
const app = new App(containerBuilder.build(), () => {
	const componentLookup = node.innerHTML.match(/\[component MockComponent\]/g);
	expect(componentLookup).not.toEqual(null);
	expect(componentLookup.length).toEqual(1);

	done();
});
...

The second parameter to App is a callback that probably gets invoked once react-habitat thinks things are done. It works fine pre-createRoot. However, swicthing to createRoot breaks this test since nothing will appear in node at the time when this callback gets executed. Wrapping the expects into a timeout makes the test pass, but the root cause probably needs to be fixed elsewhere in the library:

// tests/Bootstrapper.spec.js, it('should render a component', (done) => { ...
...
const app = new App(containerBuilder.build(), () => {
	setTimeout(() => {
		const componentLookup = node.innerHTML.match(/\[component MockComponent\]/g);
		expect(componentLookup).not.toEqual(null);
		expect(componentLookup.length).toEqual(1);

		done();
	}, 0);
});
...

Additionally, when observing the unit test output in a Karma-browser window I can see the component render. To me these findings suggest that it is the no-longer-synchronous React 18 rendering that is the problem here.

Good luck!

@wijnbladh wijnbladh changed the title Concerning React 18 and migrating to createRoot Concerning React 18 and migration to createRoot Jan 9, 2023
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

1 participant