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

Update of the @testing-library/react version to 14.0.0 throws "act" warnings #1104

Closed
maive1 opened this issue Feb 20, 2023 · 28 comments
Closed

Comments

@maive1
Copy link

maive1 commented Feb 20, 2023

  • @testing-library/react version: 14.0.0
  • Testing Framework and version: jest 29.4.2
  • DOM Environment: @testing-library/jest-dom: 5.16.5

Relevant code or config:

   const saveButton = screen.getByRole('button', { name: /save/i });
    await waitFor(() => expect(saveButton).toBeEnabled());
    await testUser.click(saveButton);

What you did:

I upgraded the version of @testing-library/react from 13.4.0 to 14.0.0. And I started having problems in all the tests of my application. Before this update this did not happen.

What happened:

I'm getting more act warnings now with @testing-library@14.0.0.
Tested with await waitFor, and still throwing act warnings with userEvent that triggers a state change.
Captura de Pantalla 2023-02-20 a la(s) 18 29 41
Captura de Pantalla 2023-02-20 a la(s) 18 29 13

Reproduction:

You can fork to reproduce the issue:
here: https://codesandbox.io/s/act-warning-wait-for-react-18-forked-6jurf2

Problem description:

The state update (setState()) seems to make the test library think that act is incomplete, even though it is. This happens for every unit test where an event is fired, such as userEvent.click() or userEvent.upload().

@eps1lon
Copy link
Member

eps1lon commented Feb 20, 2023

What happens if you wrap the timer advancement in act?

+await act(() => {
   await new Promise((resolve) => {
     setTimeout(resolve, 300);
   });
+});

@GP-marco

This comment was marked as spam.

@eps1lon
Copy link
Member

eps1lon commented Feb 21, 2023

If you're facing the same issue, please just upvote the issue or, if you want to contribute, provide a minimal, cloneable reproduction. "+1" comments are not helpful and create unnecessary notifications for everybody subscribed to this issue.

@sanderkoenders
Copy link

I have the same issue, wrapping testUser.click(saveButton) inside an act call resolves the issue. The problem is that this will have major impact on our codebase. I am wondering why this is now needed. It's not listed in the release notes as breaking change so I'd say it should not break moving from 13.x.x to 14.0.0.

@maive1
Copy link
Author

maive1 commented Feb 21, 2023

What happens if you wrap the timer advancement in act?

+await act(() => {
   await new Promise((resolve) => {
     setTimeout(resolve, 300);
   });
+});

That was the attempt to replicate the problem I have, which goes beyond that. I have problems with all the tests that are many files with unit and integration tests that have problems due to the latest version of the library, as detailed at the beginning. When I downgrade the version of the library in the package.json(), to the previous one, the problems with act() no longer appear.

@eps1lon
Copy link
Member

eps1lon commented Feb 21, 2023

I have problems with all the tests that are many files with unit and integration tests that have problems due to the latest version of the library, as detailed at the beginning.

I can't find these details. All I see is the reproduction which should be fixed by wrapping the state update in act() as explained by the warning. Unless you're saying my diff doesn't fix the issue in the repro?

@jaspalm
Copy link

jaspalm commented Feb 21, 2023

I have problems with all the tests that are many files with unit and integration tests that have problems due to the latest version of the library, as detailed at the beginning.

I can't find these details. All I see is the reproduction which should be fixed by wrapping the state update in act() as explained by the warning. Unless you're saying my diff doesn't fix the issue in the repro?

The issue we're running into is that now every test in our suite is throwing this error after upgrading when it wasn't before. Is it expected that we have to wrap every state update in act() when we did not before?

@eps1lon
Copy link
Member

eps1lon commented Feb 21, 2023

This was probably a bug that you didn't get those. If you import from @testing-library/react/pure these warnings are disabled. If you import from @testing-library/react/ we enable these warnings.

Though in this specific case I encourage you to keep them enabled since you're clearly missing to wrap state updates in act. If you have other repros where you think every state update is wrapped in act but still get warnings, please share them.

@thepuzzlemaster
Copy link

thepuzzlemaster commented Feb 23, 2023

TLDR; I have a repro where I believe every state update is wrapped in act, but still get warnings.

I've been updating my tests to account for places where I previously needed to wrap things in act() but wasn't.

There were a number places where my tests would call element.focus() and jest.advanceTimersByTime(), which trigger state updates, which were now triggering act() warnings.

I've gotten almost all my act warnings to go away by stepping through my code and tracing the state updates back to something like that. Except for a very few remaining warnings, which are all pretty similar.

I've finally found a working somewhat simplified repro (ignore the repo name).

There is one test in MyModule.test.tsx, which is testing some functionality of a custom hook.
Running npm run test runs the test, and should trigger the warning.

I can't for the life of me figure out how to avoid this act warning. As far as I can tell, the thing which triggers the state update (the timer advancing) is already wrapped in an act() function. I've tried just about every possible thing I can think of in both my actual project, and this repro, and can't figure this one out.

Not sure if it's a bug, or I'm just doing something wrong.

@skyqrose
Copy link

+1, but I'll add: I'm not just getting warnings, my tests are now failing after the upgrade. So it's not just the verbosity of the warnings that are the problem, it's the changed behavior.

I also noticed that I'm seeing these act warnings on updates I do via @testing-library/user-event.

I do like how testing-library takes care of wrapping its own calls in act in all the obvious places. I don't think expecting callers to wrap all their calls in act now is a good solution. (And if that is the plan, then it's a breaking change that would need to be in the docs.)

@mrdaniellewis
Copy link

mrdaniellewis commented Feb 26, 2023

If I've understood correctly, this is an intended breaking change, possibly due to previous unintended behaviour, and we need wrap all our calls to @testing-library/user-event in act 😞

That is super verbose. Creating an export that does within each project is probably not too hard, but it would be nice to have something that did it out of the box. Or essentially we need a @testing-library/user-event-react.

Or maybe this library could supply such a function, or re-export user-event so its calls are wrapped in act? Is that potentially on the cards?

(None of this true, ignore)

@bpinto
Copy link

bpinto commented Feb 26, 2023

I've updated the library to version 14.0.0 to fix some act warnings that were being thrown and I'm glad to report that the new release has indeed fixed them.

One shouldn't need to wrap @testing-library/user-event calls with act as that's already done for you.

I'd start with a simple test case and see if you are following all the requirements since recently there was a recent addition of setup on user-event library, or maybe you are also using an old version of react?

I could also be wrong and there might be a bug, but wanted to say that it's working as expected for me without any "missing act" being logged. It's important to see a reproducible scenario where this is happening as otherwise it's possible you are missing something on your tests setup.

@mrdaniellewis
Copy link

Thanks for confirming this is not expected. I finally managed to fix the issue on my project by deleting my package-lock.json and node-modules and re-running npm install.

@thepuzzlemaster
Copy link

thepuzzlemaster commented Feb 28, 2023

@bpinto, see my comment for a repro of (as far as i can tell), an un-fixable act warning.

@zigang93
Copy link

zigang93 commented Mar 1, 2023

base on this best pratices: https://kentcdodds.com/blog/common-mistakes-with-react-testing-library
the post mention that we don't need run cleanup, RTL will handle for us.. but not the case from my side..

I had my custom render setup https://testing-library.com/docs/react-testing-library/setup/#custom-render..
I found out that cleanup maybe not trigger in my @testing-library/[email protected]..

after I added cleanup in my global jest setup ( jest.setup.ts ) , my act warnings all gone.. hope someone can try this solution..

// jest.setup.ts
import { cleanup } from '@testing-library/react'

afterEach(() => {
  cleanup
})

@artursvonda
Copy link

I think I managed to extract the issue into reproducible test https://github.com/artursvonda/rtl-14-act-repro. Let me know if this helps. As you see in the test, I'm not doing anything funky.

@eps1lon
Copy link
Member

eps1lon commented Mar 8, 2023

@artursvonda Thank you very much for the repro. It seems to work fine with using fireEvent instead of user-event so I'm moving the issue there so that we can better identify where the issue is happening.

The first step would be to come up with a repro without any user-event code (could just inline all the necessary parts).

/cc @ph-fritsche

@eps1lon eps1lon transferred this issue from testing-library/react-testing-library Mar 8, 2023
@ph-fritsche
Copy link
Member

I see different issues being mixed up here.

  1. If you use e.g. @testing-library/react and @testing-library/user-event, those need to resolve @testing-library/dom to the same copy.

    See https://testing-library.com/docs/user-event/install:

    Note that @testing-library/user-event requires @testing-library/dom.

    If you use one of the framework wrappers, it is
    important that @testing-library/dom is resolved to the same installation
    required by the framework wrapper of your choice.
    Usually this means that if you use one of the framework wrappers, you should not
    add @testing-library/dom to your project dependencies.

    With recent versions of npm you need to run npm dedup.

  2. If you use fake timers, you need to tell @testing-library/user-event how to wind the timer.

    See https://testing-library.com/docs/user-event/options#advancetimers:

    If you are using fake timers, you need to advance your timers when we internally
    delay subsequent code.

     (delay: number) => Promise<void> | void

    default: () => Promise.resolve()

    With Jest's fake timers you need to setup user-event with {advanceTimers: jest.advanceTimersByTime}.

  3. If you get act warnings after updating React, you might have brittle test code that expects an undefined order of execution to be the same on every test run.

    See https://twitter.com/ph_fritsche/status/1626279438496280576:

    Do not just add act() calls until React shuts up!
    https://ph-fritsche.github.io/blog/post/react-act

@artursvonda
Copy link

Indeed, for my case the issue seems like was with multiple @testing-library/dom versions. Forcing v9 solved it. Thanks!

@thepuzzlemaster
Copy link

thepuzzlemaster commented Mar 8, 2023

@eps1lon, I updated my repro to use fireEvent rather than user-event. So now I have no usage of user-event at all in my repo. I also migrated it to codesandbox to lower the barrier to entry to play around with it.

I will add a couple comments to the test file to explain some of what is happening.

https://codesandbox.io/s/compassionate-marco-nekmti?file=/__tests__/MyModule.test.tsx
Running yarn test in the terminal on codesandbox triggers the one test to run - which triggers the act() warning.

@ph-fritsche
Copy link
Member

@thepuzzlemaster Your issue is explained in the blog post linked above.

    onInitiate().then((result) => { // This delays for at least one microtask...
      setId(result); // ...and then updates state.
await act(() => {
  jest.advanceTimersByTime(1000)
  return Promise.resolve() // This hides the issue in the codesandbox environment.
    // Might need any other number of microtasks in another environment though, so don't do this.
    // .then(() => void 0)
    // .then(() => void 0)
})

@thepuzzlemaster
Copy link

@ph-fritsche thanks for the added context. I read through that blog post, and this thread was also fairly helpful.

I can see how the nested onInitiate() promise adds a delay to my source code.

And as you suggest, changing my code to this:
await act(async () => jest.advanceTimersByTime(1000)) does make my act warning go away.

I wasn't entirely sure if your comments meant this was the right thing to do, or should be avoided?
When you say "// This hides the issue in the codesandbox environment", is it just masking the issue, or actually addressing the warning correctly?

And when you say "// Might need any other number of microtasks in another environment though, so don't do this", is that about returning a resolved promise in my act() call, or is that just about adding the extra promise resolutions beyond the first one.

I guess the thing I'm still not entirely clear on, is should I be adjusting my product code to make this more testable? Or is the approach here of adding a promise resolution inside my act() call the appropriate way to address this warning? Based on my (potentially incorrect) understanding, making my act an async function just pushes the warning one step down the road. And if I had another promise resolution in my source code, I'd be back in the same place?

In your blog post, you fix the issue by adding waitFor()s around your expectations, to allow the state updates to occur within those. Is that expected that me adding waitFor()s around my expectations about my spies returning does not address the state-updates in the same way?

@ph-fritsche
Copy link
Member

Just returning any promise is just masking the issue. It works with pushing it to the next microtask here, but it might fail when you run the same test in a slightly different environment. There you might need to await any other number of additional microtasks, which makes this a randomly working workaround™.

The correct way to fix with is to create a promise that resolves when the state update is applied.
Fake timers complicate this a little bit.

await waitFor(() => expect(screen.getByRole('checkbox')).toBeChecked(), {
  timeout: 10000, // Allow the callback to run ten times
  interval: 1000, // Increase the interval
})

@louis-young
Copy link

louis-young commented Mar 9, 2023

I think @ph-fritsche hit the nail on the head here 🚀

I experienced lots of erroneous act warnings after updating @testing-library/react to version 14.

  "@testing-library/react": "^14.0.0",
  "@testing-library/user-event": "^14.4.3",

I'm assuming this is due to @testing-library/react depending on @testing-library/dom version ^9.0.0, and @testing-library/user-event having a peer dependency on @testing-library/dom version >=7.21.4. This led to @testing-library/dom resolving to version 8.11.1.

  "node_modules/@testing-library/react": {
    "version": "14.0.0",
    "resolved": "https://registry.npmjs.org/@testing-library/react/-/react-14.0.0.tgz",
    "integrity": "sha512-S04gSNJbYE30TlIMLTzv6QCTzt9AqIF5y6s6SzVFILNcNvbV/jU96GeiTPillGQo+Ny64M/5PV7klNYYgv5Dfg==",
    "dev": true,
    "dependencies": {
      "@babel/runtime": "^7.12.5",
      "@testing-library/dom": "^9.0.0",
      "@types/react-dom": "^18.0.0"
    },
    "engines": {
      "node": ">=14"
    },
    "peerDependencies": {
      "react": "^18.0.0",
      "react-dom": "^18.0.0"
    }
  },
  "node_modules/@testing-library/user-event": {
    "version": "14.4.3",
    "dev": true,
    "license": "MIT",
    "engines": {
      "node": ">=12",
      "npm": ">=6"
    },
    "peerDependencies": {
      "@testing-library/dom": ">=7.21.4"
    }
  }

Overriding @testing-library/dom to version ^9.0.1 fixed all of the erroneous act warnings I was experiencing as mentioned here.

  "overrides": {
    "@testing-library/dom": "^9.0.1"
   }

Thanks for the awesome library. Hopefully this helps someone else.

@WayneEllery
Copy link

WayneEllery commented Mar 9, 2023

I think @ph-fritsche hit the nail on the head here 🚀

I experienced lots of erroneous act warnings after updating @testing-library/react to version 14.
Thanks for the awesome library. Hopefully this helps someone else.

Thanks. I experienced the same. It was caused by eslint-plugin-jest-dom which is still using version 8. For yarn I needed to add a resolution to fix it

@Chrissmith1991
Copy link

I think this may not be resolved for User Event.

// Temporary workaround for testing library's act() not working with userEvent
export async function actWrappedUserEvent(event: Promise<void>): Promise<void> {
  return await act(async () => {
    await event;
  });
}
await actWrappedUserEvent(user.click(submitButton));

the error isnt causing any tests to fail but the only way ive keep the error from bubbling up is to wrap it in an act

@robinelvin
Copy link

I suddenly started getting this issue in a previously clean project. I'm using Yarn v3 PnP with node_modules on disk.
I checked for duplicate versions of @testing-library/dom being installed and found 2 versions. I upgraded the package that was pulling in the older version and verified I now only had a single version. I was still getting warnings about using act(...) on every single test.

A clue was my CI server was not showing these warnings. In the end I had to do:

$ rm -rf node_modules yarn.lock
$ yarn install

And now my tests are working without warnings again. So I can only assume that even though Yarn thought it had cleaned everything up I still had conflicting versions on disk being referenced.

@Swappea
Copy link

Swappea commented Nov 21, 2023

Overriding @testing-library/dom to version ^9.0.1 fixed all of the erroneous act warnings I was experiencing as mentioned here.

  "overrides": {
    "@testing-library/dom": "^9.0.1"
   }

This worked for me, we are using yarn and we had to add resolutions instead of overrides.

Thank you @louis-young

"resolutions": {
    "postcss": "^8.3.6",
    "@testing-library/dom": "^9.0.1"
  }

eapearson added a commit to kbase/ui that referenced this issue May 22, 2024
multilple out-of-date transitiive dependency upon testing-library/dom leads to errors with user-event.
see testing-library/user-event#1104 (comment)
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