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" is necessary with Fake Timers even though (IIUC) shouldn't be #1218

Closed
mitchhentgesspotify opened this issue May 16, 2024 · 3 comments
Closed

Comments

@mitchhentgesspotify
Copy link

mitchhentgesspotify commented May 16, 2024

Reproduction example

https://codesandbox.io/p/devbox/9r8lr2

Prerequisites

  • The component being tested should update a useState() within a setTimeout() in response to user input
  • The test should use Fake Timers, perform that user input, then verify the results of the useState() change (e.g. querying for an element that should now be rendered)

In my linked repro, this test will fail unless the "user input" done by the test happens in an act()

So, within the linked sandbox:

  1. Run pnpm test, the test will fail
  2. Tweak MyForm.test.js and uncomment the act(...) call. Running pnpm test will now succeed.

Expected behavior

The test should pass without needing to wrap the user input in act(...), because according to this comment:

No, wrapping with act is neither required nor recommended.

Actual behavior

If the user input isn't wrapped in act(...), then, even though the useState() value was updated, a re-render isn't triggered before the test continues, causing it to fail.

User-event version

14.5.2

Environment

Repro'd in a sandbox even though it's a bit tricky these days 😅

Additional context

No response

@mitchhentgesspotify mitchhentgesspotify added bug Something isn't working needs assessment This needs to be looked at by a team member labels May 16, 2024
@pixie-cheeks
Copy link

A similar issue I encountered where the test was passing when it should fail.

App.jsx

function App() {
  const [counter, setCounter] = useState(0);
  return (
    <button
      type="button"
      // The next line should make the test fail
      onClick={() => setCounter(counter + 1)}
    >
      {counter}
    </button>
  );
}

App.test.jsx

  it('should increment a counter', async () => {
    const user = userEvent.setup();
    render(<App />);

    const button = screen.getByRole('button');
    // await act(async () => {
      await user.tripleClick(button);
    // });

    expect(button).toHaveTextContent('3');
  });

If the clicks are ever called really close to each other, it is possible that useState might use stale data. Hence, I was expecting it to catch the bug on its own since it's discouraged to wrap with act(...) according to the docs.
But unexpectedly, the test passes unless you wrap the tripleClick event with act(...).

@ph-fritsche
Copy link
Member

This is not a bug with user-event.

For the initial post by @mitchhentgesspotify see this blog post which explains why your act call seems to fix it, but only does so by chance.

For the post by @pixie-cheeks, this is an issue with the event handler:

// This will only increase by one no matter how often a user clicks between two updates of the DOM element:
<button onClick={() => setCounter(counter + 1))/>

// This will increase by one for each click no matter when the event handler on the DOM element is updated:
<button onClick={() => setCounter(prevCount => prevCount + 1)}/>

@ph-fritsche ph-fritsche closed this as not planned Won't fix, can't repro, duplicate, stale Jan 20, 2025
@ph-fritsche ph-fritsche removed bug Something isn't working needs assessment This needs to be looked at by a team member labels Jan 20, 2025
@mitchhentgesspotify
Copy link
Author

Thanks for the blog link 👍
Indeed, adding a waitFor(...) appears to fix the test without triggering warnings and without act(...).
Here's my fixed sandbox for future reference.

I went from:

  it('should', async () => {
    const user = userEvent.setup({
      advanceTimers: async delay => {
        jest.runAllTimers()
        await Promise.resolve()
      },
    })

    render(<MyForm />)
    // await act(async () => {
    await user.type(screen.getByRole('textbox'), 'a')
    // })
    console.log('Finished typing')
    screen.getByRole('button')
  })

to

  it('should', async () => {
    const user = userEvent.setup({
      advanceTimers: async delay => {
        jest.runAllTimers()
        await Promise.resolve()
      },
    })

    render(<MyForm />)
    await user.type(screen.getByRole('textbox'), 'a')
    await waitFor(() => {
      expect(screen.getByRole('button')).not.toBeNull()
    })
    console.log('Finished typing')
    screen.getByRole('button')
  })

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

3 participants