vitest-dev / vitest-browser-react

Render React components in Vitest Browser Mode
https://www.npmjs.com/package/vitest-browser-react
20 stars 1 forks source link

`act` warning when rendering #2

Open nstepien opened 2 weeks ago

nstepien commented 2 weeks ago
import { render } from 'vitest-browser-react';

import { TextButton } from './';

test('act warning', () => {
  render(<TextButton onClick={() => {}}>Button</TextButton>);
});

This is enough for me to get an act warning from React:

stderr | src/components/Button/TextButton.test.tsx > act warning
Warning: An update to %s inside a test was not wrapped in act(...).     

When testing, code that causes React state updates should be wrapped into act(...):

act(() => {
  /* fire events that update state */
});
/* assert on the output */

This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act Root

Wrapping the render call with act resolves the warning, of course:

import { act } from 'react';
import { render } from 'vitest-browser-react';

import { TextButton } from './';

test('act warning', () => {
  act(() => {
    render(<TextButton onClick={() => {}}>Button</TextButton>);
  });
});

I know the README mentions that this library does not use act, but maybe it should?

sheremet-va commented 2 weeks ago

I know the README mentions that this library does not use act, but maybe it should?

Wrapping in act is not required to work properly in the actual browser. The only difference it makes is a warning in the console because react designed it that way. We need to find a way to disable this warning.

sheremet-va commented 2 weeks ago

But indeed it may be required for render specifically because we call this method ourselves. Although the same happens in the dev environment 🤔

nstepien commented 2 weeks ago

Wouldn't it be better to use act? 🤔 As I understand it, using act ensures React fully (re-)renders and trigger all the effects following the actions that happened in its callback. Without it there may be timing issues, especially with useEffect.

sheremet-va commented 2 weeks ago

Wouldn't it be better to use act? 🤔

This is explained in the docs:

Instead, you should use Vitest's locators and expect.element API that have retry-ability mechanism baked in.

All actions should happen as they would in the browser because we don't interfere with react directly.

nstepien commented 2 weeks ago

I'm not 100% convinced this is a good idea, without act you may be testing an intermediary state rather than the final render+effect state.

All actions should happen as they would in the browser

Sure, but tests run faster than a real user would interact with a page, can we be sure that we won't miss anything without act?

sheremet-va commented 2 weeks ago

Sure, but tests run faster than a real user would interact with a page, can we be sure that we won't miss anything without act?

Yes, we can. Vitest uses CDP and webdriver to communicate with the browser, it doesn't fake events. This is how all e2e tools work, there is no need to rely on framework specific behaviours.

sheremet-va commented 1 day ago

We should wrap rendering and rerendering in the act method