Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Components which suspend do not resolve suspense when tested #9

Open
a-type opened this issue Dec 16, 2024 · 4 comments · May be fixed by #10
Open

Components which suspend do not resolve suspense when tested #9

a-type opened this issue Dec 16, 2024 · 4 comments · May be fixed by #10

Comments

@a-type
Copy link

a-type commented Dec 16, 2024

Reproduction repo here: https://github.com/a-type/vitest-browser-react-suspense-repro

I have a component which suspends on render until a certain promise is fulfilled (I'm using the new React 19 use, but I believe this behavior is consistent without React 19, for example using the older suspend-react library).

When I render this component in a test (wrapping it with a Suspense boundary), it never updates. I can see that the promise is resolved via the test log, but the rendered output remains in the suspended state.

If I run a normal Vite app with the same component, I can see that after the appropriate time, the suspense is resolved and the component contents are displayed.

Additional logs from the test are probably relevant:

A component suspended inside an `act` scope, but the `act` call was not awaited. When testing React components that depend on asynchronous data, you must await the result:

await act(() => ...)

I manually patched the vitest-browser-react module to await act on initial render and it appears to have resolved the problem. However, this changes the API, requiring render to be an async function and awaited by the user.

-function act(cb) {
+async function act(cb) {
	const _act = React.act || React.unstable_act;
	if (typeof _act !== "function") {
		cb();
	} else {
		globalThis.IS_REACT_ACT_ENVIRONMENT = true;
-		_act(cb);
+		await _act(cb);
		globalThis.IS_REACT_ACT_ENVIRONMENT = false;
	}
}

// ...
-function render(ui, { container, baseElement, wrapper: WrapperComponent } = {}) {
+async function render(ui, { container, baseElement, wrapper: WrapperComponent } = {}) {

// ...
-act(() => {
+await act(() => {
	root.render(
		strictModeIfNeeded(wrapUiIfNeeded(ui, WrapperComponent))
	);
});
@sheremet-va
Copy link
Member

I am OK with changing the API. @MatanBobi what do you think?

@MatanBobi
Copy link
Contributor

It's reasonable to me, we have an effort for that in RTL: testing-library/react-testing-library#1214 but it's been open for a while now. I believe that the main concern is that since we're not wrapping interactions here with act, what will it mean if a click triggers a transition for example?

@sheremet-va
Copy link
Member

Vitest triggers a native click, so it should work the same way it works in production. To catch the changes in DOM, users should call page.getBy* locators that will wait for the presence of the element

@MatanBobi
Copy link
Contributor

So I believe that this change will happen sooner or later in RTL too so I'm all in on changing it here too.

@MatanBobi MatanBobi linked a pull request Dec 17, 2024 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants