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

act warning when rendering #2

Open
nstepien opened this issue Sep 13, 2024 · 6 comments
Open

act warning when rendering #2

nstepien opened this issue Sep 13, 2024 · 6 comments

Comments

@nstepien
Copy link
Contributor

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

sheremet-va commented Sep 13, 2024

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

sheremet-va commented Sep 13, 2024

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

@nstepien
Copy link
Contributor Author

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

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

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

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.

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