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

The current testing environment is not configured to support act(...) with vitest and React 18 #1061

Closed
MarkLyck opened this issue May 3, 2022 · 21 comments · Fixed by #1070
Labels
needs discussion Whether changes are needed is still undecided and additional discussion is needed.

Comments

@MarkLyck
Copy link

MarkLyck commented May 3, 2022

"@testing-library/jest-dom": "5.16.4",
"@testing-library/react": "13.1.1",
DOM Environment: jsdom or happy-dom (happens with both)

Problem description:

After upgrading to @testing-library/react@13.x and react@18.x with vitest. A lot of react tests throw the following warning:

Warning: The current testing environment is not configured to support act(...)

I noticed this after switching from jest to vitest, and not changing anything else. (The tests do not show this warning when using jest, but they they show it when using vitest in the same project).

I previously had an unrelated issue causing the same warning with Jest: #1057

I resolved that, by removing a bad implementation of act(). However this time the tests are identical, and works with 1 test-runner and not with another.

Reproduction:

Credit to @stuarthallows for creating a repo that reproduces the problem: https://github.com/stuarthallows/vitest-error

@stuarthallows and @dar5hak also both seem to be having the same problem as me (see my other issue here: #1057 for more details).

Additional information.

In my case, this is breaking unit tests completely because it logs thousands of these warnings and it just stops after around 10 tests are completed.

For me a lot of the errors start with rc-motion

Warning: The current testing environment is not configured to support act(...)
    at Align (/Users/marklyck/colony/colony-frontend/node_modules/.pnpm/rc-align@4.0.12_ef5jwxihqo6n7gxfmzogljlgcm/node_modules/rc-align/lib/Align.js:45:23)
    at DomWrapper (/Users/marklyck/colony/colony-frontend/node_modules/.pnpm/rc-motion@2.5.1_ef5jwxihqo6n7gxfmzogljlgcm/node_modules/rc-motion/lib/DomWrapper.js:28:34)
    at /Users/marklyck/colony/colony-frontend/node_modules/.pnpm/rc-motion@2.5.1_ef5jwxihqo6n7gxfmzogljlgcm/node_modules/rc-motion/lib/CSSMotion.js:57:32

Which is probably running an animation with antd components. But I'm 99% sure that's a red-herring since the same tests and components work just fine with jest. (or testing-library/react@12 and react@17)

@MarkLyck
Copy link
Author

MarkLyck commented May 4, 2022

I found a workaround that seems to work.

If you manually add

global.IS_REACT_ACT_ENVIRONMENT = true

To the setupFile the warning will go away.

This should of course be handled by React testing library so it's not a proper fix. But it allows us to use vitest until this issue is resolved :)

@airjp73
Copy link

airjp73 commented May 5, 2022

The workaround doesn't totally work for me. The initial error The current testing environment is not configured... to support act(...) goes away, but then I get act warnings I wouldn't have otherwise.

I've made a reproduction here: https://github.com/airjp73/vitest-act-reproduction

Worth noting that this also appears to be exclusive to React 18. If you downgrade everything to React 17 (including downgrading this library to v12), the act warnings go away and you also don't need to override the global anymore.

@nknapp
Copy link

nknapp commented May 6, 2022

I raised a similar issue at vitest. But they think (and I kind of agree), that it has to be solved in testing-library.

tldr; of my vitest-issue:

  • when testing-library/react resolves IS_REACT_ACT_ENVIRONMENT it uses the first existing of self, window and global as global-object, which in may case is self
  • react access IS_REACT_ACT_ENVIRONMENT directly, which is probably globalThis or global link.
  • in vitest / jsdom, globalThis and self are not the same object.

The fix

I think the fix is to add

if (typeof globalThis !== 'undefined') {
  return globalThis
}

here:

function getGlobalThis() {
/* istanbul ignore else */
if (typeof self !== 'undefined') {
return self
}
/* istanbul ignore next */
if (typeof window !== 'undefined') {
return window
}
/* istanbul ignore next */
if (typeof global !== 'undefined') {
return global
}
/* istanbul ignore next */
throw new Error('unable to locate global object')
}
function setIsReactActEnvironment(isReactActEnvironment) {
getGlobalThis().IS_REACT_ACT_ENVIRONMENT = isReactActEnvironment
}
function getIsReactActEnvironment() {
return getGlobalThis().IS_REACT_ACT_ENVIRONMENT
}
.

The workaround

The workaround is define some kind of proxy-property on global this, that will make sure that self and globalThis have the same value of IS_REACT_ACT_ENVIRONMENT the code in setupFile.ts:

Object.defineProperty(globalThis,"IS_REACT_ACT_ENVIRONMENT", {
  get() {
    if (typeof globalThis.self !== 'undefined') {
      return globalThis.self.IS_REACT_ACT_ENVIRONMENT
    }
  },
  set(value) {
    if (typeof globalThis.self !== 'undefined') {
      globalThis.self.IS_REACT_ACT_ENVIRONMENT = value
    }
  }
})

globalThis.IS_REACT_ACT_ENVIRONMENT = true

@eps1lon
Copy link
Member

eps1lon commented May 7, 2022

We're waiting on confirmation for vitest-dev/vitest#1146 (comment). We're matching the globalThis polyfill so it isn't obvious that we can or should change something.

@eps1lon eps1lon added the needs discussion Whether changes are needed is still undecided and additional discussion is needed. label May 7, 2022
@nknapp
Copy link

nknapp commented May 13, 2022

shouldn't a polyfill only apply if the feature itself is not available? I would argue that if globalThis exists, you should use
it, and not the polyfill.

@eps1lon
Copy link
Member

eps1lon commented May 13, 2022

shouldn't a polyfill only apply if the feature itself is not available? I would argue that if globalThis exists, you should use it, and not the polyfill.

We can do that. The code shouldn't necessarily be considered a polyfill. It worked perfectly well when globalThis didn't exist. So there's definitely something wrong in vitest environments since they wouldn't support legacy code that works in any other environment.

We can still fix it by using globalThis if it exists. So feel free to send a PR. But older versions will continue to not work in vitest until they fix that issue on their side as well.

@airjp73
Copy link

airjp73 commented May 13, 2022

I haven't had a chance to test myself, but I believe this was fixed on vitest's end.

@ulrichstark
Copy link

Yes can confirm that this issue is fixed for me with newest vitest version.

@WonderPanda
Copy link

I've upgraded to the latest version of vitest (0.12.6 at the time of writing) and it still doesn't resolve the issue. Using the workaround @nknapp here does successfully get rid of the warnings

@stuarthallows
Copy link

stuarthallows commented May 14, 2022

Also not fixed for me with vitest 0.12.6, Nor does the workaround prevent the warnings.

@nknapp
Copy link

nknapp commented May 28, 2022

Thanks for creating that MR, @eps1lon. I think the issue I mentioned is indeed fixed with vitest@0.12.6

@stuarthallows I have just spent a couple of hours trying to find out why I also still get some act-warnings. Maybe you have a similar problem.

The following code is wrong, but the error is very hard to see

it("sends no access token after logout", async() => {
    // ...
    await clickLogoutButton();
    await waitForLogout();
    // ...
})

async function clickLogoutButton() {
  await act(() => {
    return screen.getByText("Logout").click();
  });
}

There is an await act(), but the callback is a synchronous function. This leads to some bad "timing", so that the actual state-change is just after the act() call, but before the waitFor-call that is done inside waitForLogout()

The function clickLogoutButton should be either completely synchronous:

it("sends no access token after logout", async() => {
    // ...
    clickLogoutButton();  // <---- no "await"
    await waitForLogout();
    // ...
})

function clickLogoutButton() {  // <--- no "async"
  act(() => {  // <--- no "await"
    screen.getByText("Logout").click();
  });
}

or completely asynchronous

it("sends no access token after logout", async() => {
    // ...
    await clickLogoutButton(); 
    await waitForLogout();
    // ...
})

async function clickLogoutButton() {
  await act(async () => { // <--- added "async" to callback
    screen.getByText("Logout").click();
  });
}

Update: There are some eslint-rules, for TypeScript, that can detect such errors. I added the rules require-await and @typescript-eslint/await-thenable to my project.

@mindplay-dk
Copy link

There is an await act(), but the callback is a synchronous function. This leads to some bad "timing", so that the actual state-change is just after the act() call, but before the waitFor-call that is done inside waitForLogout()

This paragraph right here was a life-saver after two solid days of fighting through a react 18 upgrade.

Apparently, if you put find* calls inside an act, the timing somehow doesn't work out... it would be reasonable to think "more awaits better" in terms of timing, but for some reason, awaiting an act that awaits things doesn't work out well.

The results are extremely confusing and spooky - literally your test can pass and then error after they pass!

$ yarn test -- pages/Home
yarn run v1.22.19
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.
$ jest pages/Home
 PASS  src/pages/Home/index.test.tsx
  ● Console

    console.error
      Warning: The current testing environment is not configured to support act(...)

This particular project is open source, and you can see the commit that fixed my failing tests here - it has several examples of "things that don't work if you wrap them in act":

abtion/muffi.net@b70302f

Simply removing some of the act wrappers fixes the timing issues.

This should be covered by documentation maybe?

I know you're just pulling this functionality from react's own facilities, but their documentation doesn't cover this - and I suppose maybe their documentation can't be expected to cover the interplay with facilities provided by react-testing-library?

Not sure. This was extremely confusing and time consuming and seems like something almost everyone would run into at first.

Could it be detected and guarded against, with a better error message to guide people along maybe? 🤔

@targumon
Copy link

targumon commented Sep 6, 2022

With React 17 + @TL/react 12 + @TL/user-event 13 I had tests such as this:

it('handles clicks', () => {
  const spy = jest.fn()
  render(<Button onClick={spy}/>)
  userEvent.click(screen.getByRole('button'))
  expect(spy).toBeCalled()
})

Worked just fine. I was proud of not using act even once in the entire project.
(btw, note my test is synchronous. async/await was only needed with more complex components)

I'm working on upgrade to React 18 + @TL/react 13 + @TL/user-event 14 👷
Following the recommendation in the docs I switched out of userEvent:

const user = userEvent.setup()
await user.click(screen.getByRole('button')) // ❗️jest throws timeout error

For most (simple) tests, this was fixed by adding waitFor:

await waitFor(() => user.click(screen.getByRole('button')))

However, the more complex components would throw 'An update to Comp inside a test was not wrapped in act'.
Seems weird because I was under the impression the waitFor does that under the hood.
Swallowed my pride 😞 and fixed it by adding act:

await act(() => waitFor(() => user.click(screen.getByRole('button')))) // not as pretty as before, but works

Surprisingly in some cases I get 'The current testing environment is not configured to support act' (sadly I can't tell why it warns for certain components but not for others).
After hours of trying to fix it I found a solution completely by accident:

await act(() => waitFor(() => { user.click(screen.getByRole('button')) })) // ✅ curly braces!

I hope this is helpful - either to the wonderful maintainers of TL or any user facing the same warning. 🙏

EDIT: I'm aware the callback in waitFor could be either callback: () => (void | Promise<void>). Still wrapping my head around all of this.

UPDATE: Some of my issues were because I useFakeTimers - this was solved by userEvent.setup({advanceTimers: jest.advanceTimersByTime}). I still faced many "testing environment is not configured to support act" warnings (mainly in components that use react-query 🤔) and ended up overriding console.error to ignore them.

FINAL(?) EDIT (2023-02-06): All of the above is one long workaround for an error caused by me not properly waiting for my components to load. Don't do it. Read my other comment.

Please don't do this! Read another comment I left below👇

@PenghaiZhang
Copy link

My case is if I run my test individually everything is good. But I run all of my tests at the same time this error comes up.
I have userEvent.setup({advanceTimers: jest.advanceTimersByTime}) in the test that needs some fake timing.

@binhphan009
Copy link

binhphan009 commented Feb 2, 2023

thanks @targumon

await act(() => waitFor(() => { user.click(screen.getByRole('button')) })) // ✅ curly braces!

saved my life <3

@nknapp
Copy link

nknapp commented Feb 3, 2023

Tbh. I don't think it's good practice to put actions like 'click' into a 'waitfor'.

waitFor is for observation. It may end up in multiple clicks if you are not careful.

waitFor should also not be nested in 'act'.

Those may be pragmatic workarounds, but not proper solutions.

I am writing this for everybody who is new to testing-library, reading this thread and learning from it.

If these problems still occur, it's probably a bug, in testing-library or vitest.

@IanTorquato
Copy link

IanTorquato commented Feb 3, 2023

I don't know if it helps, but I had the same error without using Vitest
Warning: The current testing environment is not configured to support act(...)

And global.IS_REACT_ACT_ENVIRONMENT = true; didn't work for me

@targumon
Copy link

targumon commented Feb 6, 2023

saved my life <3

@binhphan009 You're welcome, but in fact, a couple of months after I left that comment here, I re-visited my test suites and removed almost all of the act calls.

It turns out I wasn't properly waiting for my API-dependent components to load (I use react-query and I obviously mock the backend in these tests, but the wait still needed to be handled) so jest timed out because the UI wasn't ready.

Wrapping user.click in a waitFor and then again in an act is a bad workaround (as @nknapp rightfully points out).
btw, the curly braces are ANOTHER bad workaround: they just cause the callback provided to waitFor to NOT return the promise from user.click which slightly alters its behavior in our favor, but it's just an accident, not a proper solution.

I'm now back to using await user.click(screen.getByRole('button')) totally error-free.

@sag1v
Copy link

sag1v commented Jul 5, 2023

I'm now back to using await user.click(screen.getByRole('button')) totally error-free.

@targumon So what were your findings and how did you resolve it? 😄

@nickmccurdy nickmccurdy pinned this issue Sep 11, 2023
@nickmccurdy nickmccurdy unpinned this issue Sep 11, 2023
@nikolyachikm
Copy link

had this error which randomly failed tests. in my case I had code like this (select an option in dropdown):

fireEvent.mouseDown(...);
fireEvent.click(...);

solution:

await waitFor(() => {
    fireEvent.mouseDown(...);
    fireEvent.click(...);
});

@KimSchneider
Copy link

KimSchneider commented Oct 5, 2023

I'm now back to using await user.click(screen.getByRole('button')) totally error-free.

@targumon So what were your findings and how did you resolve it? 😄

@targumon you teased an answer but I am missing code! 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Whether changes are needed is still undecided and additional discussion is needed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.