Skip to content

Warning about using different versions of act() when wait()ing on an async effect #173

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

Closed
pelotom opened this issue Sep 15, 2019 · 15 comments · Fixed by #200
Closed

Warning about using different versions of act() when wait()ing on an async effect #173

pelotom opened this issue Sep 15, 2019 · 15 comments · Fixed by #200
Labels
enhancement New feature or request

Comments

@pelotom
Copy link

pelotom commented Sep 15, 2019

  • @testing-library/react-hooks version: 2.0.1
  • react-test-renderer version: 16.9.0
  • react version: 16.9.0
  • node version: 12.9.0
  • npm (or yarn) version: 1.17.3 (yarn)

Relevant code or config:

import { useState, useCallback, useEffect } from 'react';
import { renderHook, act } from '@testing-library/react-hooks';
import { wait } from '@testing-library/react';

test('should increment counter', async () => {
  const { result } = renderHook(useCounter);

  act(result.current.increment);

  expect(result.current.count).toBe(1);

  await wait(() => expect(result.current.count).toBe(2));
});

function useCounter() {
  const [count, setCount] = useState(0);

  useEffect(() => {
    setTimeout(() => {
      setCount(count => count + 1);
    });
  }, []);

  const increment = useCallback(() => setCount(x => x + 1), []);

  return { count, increment };
}

What you did:

Tried to augment the example test in the README with an asynchronous effect.

What happened:

Although the test passes I get a warning about using differing versions of act():

 PASS  test/foo.test.tsx
  ✓ should increment counter (13ms)

  console.error node_modules/react-test-renderer/cjs/react-test-renderer.development.js:104
    Warning: It looks like you're using the wrong act() around your test interactions.
    Be sure to use the matching version of act() corresponding to your renderer:

    // for react-dom:
    import {act} from 'react-dom/test-utils';
    //...
    act(() => ...);

    // for react-test-renderer:
    import TestRenderer from 'react-test-renderer';
    const {act} = TestRenderer;
    //...
    act(() => ...);
        in TestHook
        in Suspense
@pelotom pelotom added the bug Something isn't working label Sep 15, 2019
@mpeyper
Copy link
Member

mpeyper commented Sep 15, 2019

Hi @pelotom,

The issue here is that you are using wait from @testing-library/react which uses render from react-dom and act from react-dom/test-utils while this library instead uses create and act from react-test-renderer and doesn't use react-dom, @testing-library/react or @testing-library/dom at all for rendering (there are reasons why we prefer this approach).

You could follow our docs on how to handle async with the waitForNextUpdate helper. I'd also be willing to consider PRs to add similiar sorts of utilities that come from @testing-library/react and/or @testing-library/dom so things feels a bit more seemless moving between the different testing libraries.

@mpeyper mpeyper added enhancement New feature or request and removed bug Something isn't working labels Sep 15, 2019
@mwmcode
Copy link

mwmcode commented Jan 29, 2020

Thanks @mpeyper

I don't get why someone would downvote an answer that both explains the issue and provides a solution! ¯_(ツ)_/¯

@mpeyper
Copy link
Member

mpeyper commented Jan 29, 2020

Haha, yeah... not sure either.

I guess I'll just have to issue them a refund for the licensing fees to use the library 😅

@tyeon95
Copy link

tyeon95 commented May 16, 2020

Hi @pelotom,

The issue here is that you are using wait from @testing-library/react which uses render from react-dom and act from react-dom/test-utils while this library instead uses create and act from react-test-renderer and doesn't use react-dom, @testing-library/react or @testing-library/dom at all for rendering (there are reasons why we prefer this approach).

You could follow our docs on how to handle async with the waitForNextUpdate helper. I'd also be willing to consider PRs to add similiar sorts of utilities that come from @testing-library/react and/or @testing-library/dom so things feels a bit more seemless moving between the different testing libraries.

I've followed the documentation for handling async hooks.

But I'm still receiving the same warning that I'm using act() even though I'm no longer using act() in the test function now as per the documentation, Warning: It looks like you're using the wrong act() around your test interactions. Be sure to use the matching version of act() corresponding to your renderer:
Is there anything I'm missing?

  • @testing-library/react-hooks version: ^16.13.1
  • react-test-renderer version: 16.13.1
  • react version: ^16.13.1
  • @testing-library/react version: ^9.3.2
describe('hooks', () => {
  it('getProjects', async () => {
    const { result, waitForNextUpdate } = renderHook(() => useProjects());

    expect(Array.isArray(result.current.projects)).toBeTruthy();
    expect(result.current.projects.length).toBe(0);

    result.current.getProjects();
    await waitForNextUpdate();

    expect(result.current.projects.length > 0).toBeTruthy();
  });
});

@mpeyper
Copy link
Member

mpeyper commented May 21, 2020

Hi @tyeon95,

Sorry to hear you're also having problems with and act warning.

Everything in the test you've shared looks to correct, so I'm not sure what this issue might be. Are you able to share the code for your useProjects hook as well, and any other code in the test files that may be relevant (e.g. mocking api calls, etc.)?

Also, you've listed the react version for @testing-library/react-hooks instead of the version for this library. Can you please share the correct version for that as well?

Last thing. The version for react-test-renderer you specified does not have the ^ modifier, like the others. Was that just missed when leaving your message, or is is specified without it in your package.json as well? You want that be be identical to your react dependency version to make sure they stay in sync as new versions are released.

@millievn
Copy link

some problem

  • "@testing-library/react": "^10.0.3",
  • "@testing-library/react-hooks": "^3.2.1"
  • "react-test-renderer": "^16.13.1",
  • "react": "^16.13.1"
  • node v12.16.1

demo like this

// api.ts
import { message } from 'antd'
import { useHistory } from 'react-router-dom'
//  generated by graphql-code-generator
// https://graphql-code-generator.com/docs/plugins/typescript-react-apollo
import { useLoginMutation } from './generated'

function useLoginApi() {
  const history = useHistory()
  const [loginMutation, { loading }] = useLoginMutation()

  const submit = async (pwd: string, phone: string) => {
    try {
      const { data } = await loginMutation({
        variables: {
          phone,
          pwd,
        },
      })
      if (data?.login?.role !== 5) {
        message.error('no auth')
        return
      }
      message.success('success')
      history.replace('/home')
    } catch (e) {
      message.error('error')
    }
  }

  return {
    loading,
    submit,
  }
}


// api.test.tsx
import React from 'react'
import { MemoryRouter as Router } from 'react-router-dom'
import { renderHook, act } from '@testing-library/react-hooks'
import { MockedProvider, MockedResponse } from '@apollo/react-testing'
import { useLoginApi } from './api'
import { LoginDocument } from './generated'

interface MockedProps {
  children?: any
}

const mooks: MockedResponse[] = [
  {
    // success
    request: {
      query: LoginDocument,
      variables: { phone: 18812345678, pwd: '123456' },
    },
    result: {
      data: {
        login: {
          id: 'id',
          token: 'token',
          role: 5,
          nickname: 'nickname',
        },
      },
    },
  },
]


it('useLoginApi', async () => {
    const { result, waitForNextUpdate } = renderHook(() => useLoginApi(), {
      wrapper: ({ children }: MockedProps) => (
        <Router>
          <MockedProvider mocks={mooks}>{children}</MockedProvider>
        </Router>
      ),
    })

    expect(result.current.loading).toEqual(false)

    //  Warning: An update to TestHook inside a test was not wrapped in act(...).
    act(() => {
      result.current.submit('123456', '18812345678')
    })

    expect(result.current.loading).toEqual(false)
  })

it('useLoginApi', async () => {
    const { result, waitForNextUpdate } = renderHook(() => useLoginApi(), {
      wrapper: ({ children }: MockedProps) => (
        <Router>
          <MockedProvider mocks={mooks}>{children}</MockedProvider>
        </Router>
      ),
    })

    expect(result.current.loading).toEqual(false)

    result.current.submit('123456', '18812345678')

     //  Warning: An update to TestHook inside a test was not wrapped in act(...).
     await waitForNextUpdate()

    expect(result.current.loading).toEqual(false)
  })

it('useLoginApi', async () => {
    const { result, waitForNextUpdate } = renderHook(() => useLoginApi(), {
      wrapper: ({ children }: MockedProps) => (
        <Router>
          <MockedProvider mocks={mooks}>{children}</MockedProvider>
        </Router>
      ),
    })

    expect(result.current.loading).toEqual(false)

    result.current.submit('123456', '18812345678')

     //   Warning: It looks like you're using the wrong act() around your test interactions.
    act(() => {
      result.current.submit('123456', '18812345678')
    })

     await waitForNextUpdate()

    expect(result.current.loading).toEqual(false)
  })

@mpeyper
Copy link
Member

mpeyper commented May 22, 2020

I've been able to reproduce this with the help of @qiwang97's example (I had to make up what was in generated.ts myself, so it may not be 100% right, but it does present the issue, so I'm not too worried.

You can check out the sandbox here if you want to double check anything or make any suggestions for things to change.

The console output clearly shows the warning message being presented on the await waitForNextUpdate() line in the test, which is super frustrating because that is definitely using act from react-test-reneder, so I'm unsure why the warning is being presented.

Interesting, as the example actually returns the promise, it is possible to write this test to avoid the use of waitForNextUpdate altogether by using the async act variant:

expect(result.current.loading).toEqual(false)

await act(() => result.current.submit('123456', '18812345678'))

expect(result.current.loading).toEqual(false)

This is basically the same way that waitForNextUpdate wraps the promise that resolves when the component renders next, and it too produces the warning about using the wrong act.

It's late now, so I'm going go to bed now, but next time I have some time (likely sometime this weekend), I'm going to set up a basic example that renders a component with a hook using pure react-test-renderer calls, and make an update using the async act variant to see if it's actually just react-test-renderer acting up on us (pun intended).

Annoyingly, I don't see the same warning when I run the test suite for this library, so I don't think I'll see the warning when just using pure react-test-renderer calls in a basic example, and it instead has something to do with the way the react-test-renderer is being resolved for the package when it is installed as a node_module, but I can't really see how that could be the case.

@mpeyper mpeyper reopened this May 22, 2020
@mpeyper
Copy link
Member

mpeyper commented May 23, 2020

Ok @qiwang97, got something, but not sure what it means yet. If I comment out the antd calls to message, the warning goes away. See this sandbox where that is the only change.

I've taken a look at the antd code, and it's not immediately obvious why the message calls are presenting this warning, but it's feeling like more of an issue with that package and however message works than something this library is doing wrong with act calls.

@mpeyper
Copy link
Member

mpeyper commented May 23, 2020

I've yet to confirm it (out of time today), but I believe it's happening because the underlying rc-notification package that the antd message functionality uses is implicitly using react-dom for rendering, while this library uses react-test-renderer, so while we are acting for our own rendered components, the warning is coming from react-dom saying you're using the wrong act function for whatever it's doing.

@mpeyper
Copy link
Member

mpeyper commented May 24, 2020

Ok, can confirm, if I hack my node_modules to add console logs on either side of the ReactDom.render call in rc-notification, then they appears on either side of the act warning.

Interestingly, it appears as though they have the functionality to provide an alternative TEST_RENDER function, however it does not appear as though antd provides any ability to set it in their message functionality when creating the Notification instance.

I'm closing this now as I cannot force every library to never call ReactDOM.render and my ability to resolve issues in those other libraries is quite limited. I suggest you raise issues over there, but honestly, there is very little incentive for them to make changes for a relatively niche requirement of popping a notification in a custom hook being called in a test using react-test-renderer. Potentially it is just easier to let your hook override the message instance and provide a mock in your test:

// api.ts
import { message as antdMessage } from 'antd'
import { useHistory } from 'react-router-dom'
//  generated by graphql-code-generator
// https://graphql-code-generator.com/docs/plugins/typescript-react-apollo
import { useLoginMutation } from './generated'

function useLoginApi({ message = antdMessage } = {}) {
  const history = useHistory()
  const [loginMutation, { loading }] = useLoginMutation()

  const submit = async (pwd: string, phone: string) => {
    try {
      const { data } = await loginMutation({
        variables: {
          phone,
          pwd,
        },
      })
      if (data?.login?.role !== 5) {
        message.error('no auth')
        return
      }
      message.success('success')
      history.replace('/home')
    } catch (e) {
      message.error('error')
    }
  }

  return {
    loading,
    submit,
  }
}

@mpeyper mpeyper closed this as completed May 24, 2020
@millievn
Copy link

@mpeyper Thanks a lot and I will try.

@mayacode
Copy link

@mpeyper could this be related ? Unfortunately works only on chrome, on ff codesandbox has some error.

Screenshot 2020-08-18 at 18 02 27

@mpeyper
Copy link
Member

mpeyper commented Aug 18, 2020

yes @mayacode, this appears to be the exact same issue with antd using rc-notification for the notification functionality you're calling in useManageNotifications. You can see that the call to notification is producing the warning by logging either side of it and observing the order of logs in the console:

image

@mayacode
Copy link

Thanks a lot, I started to align our test and I mock the problematic stuff. It needs some additional work but I decreased the number of warnings with keeping the test coverage. Thank you again!

@mwanagosos
Copy link

For me, using

const { waitFor } = renderHook(() =>
  // ...
);

instead of

import { waitFor } from '@testing-library/react';

solved the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants