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

new act warning since update #535

Closed
donaldpipowitch opened this issue Nov 18, 2019 · 20 comments
Closed

new act warning since update #535

donaldpipowitch opened this issue Nov 18, 2019 · 20 comments

Comments

@donaldpipowitch
Copy link

donaldpipowitch commented Nov 18, 2019

  • react-testing-library version: 9.3.1
  • react version: 16.12.0
  • node version: 10.16.0
  • yarn version: 1.19.1

Relevant code or config:

https://github.com/donaldpipowitch/testing-library-react-act-bug-demo

What you did:

Updated @testing-library/react from 9.3.0 to 9.3.1. Also see #514 (afaik the only change in that version bump).

What happened:

A new warning appeared and I don't know, if this is a bug or not.

Reproduction:

https://github.com/donaldpipowitch/testing-library-react-act-bug-demo

Note that using an "await" somewhere can make the error go away, so this might be some race condition thingy with the event loop.

Problem description:

A new warning appeared and I don't know, if this is a bug or not.

Suggested solution:

If it's not a bug, but intended - everything is fine. If it's a bug, the warning should disappear after the bug fix.

@threepointone
Copy link
Contributor

I'll have a look at this next week.

@ghargrove
Copy link

@donaldpipowitch @threepointone I don't have much background info here but I think it's possible that something within formik may be causing this issue?

I encountered the same (or similar?) error (below) when I initially built my form with the formik useFormik hook. As a test I swapped out useFormik with a useState hook and the test stopped throwing the error.

As an additional test I started checking previous versions of @testing-library/react and it appears the issue began w/ 9.3.1. The test did not throw the error while using formik w/ react-testing-library 9.3.0.

Hope that's helpful.

  • @testing-library/react version 9.3.2
  • @apollo/react-testing version 3.1.3
  • react version 16.12.0
  • formik version 2.0.6
  • node version 12.13.0
  • yarn version 1.19.1
console.error node_modules/react-dom/cjs/react-dom.development.js:530
    Warning: An update to Login 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://fb.me/react-wrap-tests-with-act
        in Login
        in ApolloProvider (created by MockedProvider)
        in MockedProvider

@donaldpipowitch
Copy link
Author

but I think it's possible that something within formik may be causing this issue?

Yeah, it's likely. I personally don't know if the warning is correct or a "false negative".

@ghargrove
Copy link

ghargrove commented Nov 27, 2019

@donaldpipowitch My previous comment may not be accurate about it involving formik. After hacking a bit more I was able to get my tests to pass w/o the error being thrown.

My first problem was that I was incorrectly using the apollo mock provider and had the result returning wrong.

Secondly I wasn't calling any of the wait* variations to allow the mocked api request to resolve.

Here was my particular test after I got it fixed. Hope it helps.

import React from 'react';

import { MockedProvider } from '@apollo/react-testing';
import '@testing-library/jest-dom/extend-expect';
import { fireEvent, render, waitForElement } from '@testing-library/react';

import LoginPage, { loginMutation } from './Login';

it('renders a message if the login is not successful', async () => {
  const email = '[email protected]';
  const password = 'badpassword';
  const mocks = [
    {
      request: {
        query: loginMutation,
        variables: {
          email,
          password
        }
      },
      result: { data: { loginUser: null } }
    }
  ]

  const { getByLabelText, getByTestId, getByText } = render(
    <MockedProvider mocks={mocks} addTypename={false}>
      <LoginPage />
    </MockedProvider>
  );
    
  fireEvent.change(getByLabelText('Email:'), {
    target: { value: email },
  });
  fireEvent.change(getByLabelText('Password:'), {
    target: { value: password },
  });
  fireEvent.click(getByText('Login', { selector: 'button' }));

  const messageEl = await waitForElement(() => getByTestId('message'));
  expect(messageEl).toHaveTextContent(
    'Invalid email and/or password'
  );
});

@ghargrove
Copy link

ghargrove commented Nov 27, 2019

@donaldpipowitch as a follow up to my comment above. I added an await to your example repo w/ "@testing-library/react": "^9.3.2", and was able to get the tests to pass

import React from 'react'
import {render, fireEvent, waitForElement} from '@testing-library/react'
import { Formik, Form, useField } from 'formik';

const Input = ({ label, name }) => {
  const [field] = useField(name);

  return (
    <label>
      {label}

      <input {...field} />
    </label>
  );
};

test('formik example', () => {
  const { getByLabelText } = render(
    <Formik
      initialValues={{
        email: ''
      }}
      onSubmit={() => {}}
    >
      <Form>
        <Input name="email" label="Email address" />
      </Form>
    </Formik>
  );

  fireEvent.change(getByLabelText('Email address'), {
    target: { value: '[email protected]' }
  });

  waitForElement(() => getByLabelText('Email address')).then(el => {
    expect(el).toBeDefined();
    expect(el.value).toEqual('[email protected]');
  })
  
})

@donaldpipowitch
Copy link
Author

@ghargrove Sorry that I haven't pointed it out in the description that using an "await" will make this warning go away. I noticed it as well when I initially found the error here: #514 (comment). I'll update the description.

@ghargrove
Copy link

ghargrove commented Nov 29, 2019

I'm fairly confident the issue I was encountering was #224.

After my updates in my previous comment I continued to have tests that would sometimes run without the errors and other times would throw them. I couldn't shake the feeling it was related to formik as they'd stop every time I pulled it out.

I had to tweak my tests a bit but now I've got them running without issues.

One of the biggest things that took me a minute to realize was that my component implementation was doing <div data-testid="message">{error && 'Invalid login'}</div> instead of {error && <div data-testid="message">Invalid login</div>}. Therefore await findByTestId('message') was always resolving before all of the async actions has a chance to take place

@jednano
Copy link

jednano commented Dec 10, 2019

I'm getting the same (red) warning on v9.3.2 and even 9.3.0 if I downgrade. Not using formik at all here. The issue appears to be 100% related to a dispatch call within a useEffect.

const Foo: React.FC = () => {
  const [, api] = someContext.use();

  useEffect(() => {
    let didCancel = false;
    api.fetchSomething(() => didCancel);
    return () => {
      didCancel = true;
    };
  }, [api]);

  return null;
};

I even simplified the fetch call to this simple Promise.resolve:

export default async function fetchSomething() {
  return Promise.resolve([]);
}

Still, the same (red) warning.

@AriPerkkio
Copy link
Contributor

+1, seeing this error on 9.40 and 9.32. Downgrading to 9.3.0 fixes the issue. My setup is just react without external dependencies.

@gnapse
Copy link
Member

gnapse commented Jan 16, 2020

I can confirm that sticking to 9.3.0 makes me not have the warning, while 9.3.1 does.

Things I did:

  • Noticed the warning when upgrading to v9.4 from 8.x
  • Found this issue and the last comment
  • Downgraded to v9.3.0 and the warning disappeared
  • Upgraded to 9.3.1 and the message re-appeared

@kentcdodds
Copy link
Member

I'm 99% certain that in this case y'all are suffering from the thing I discuss in the appendix of this blog post. TL;DR: act is working properly (as is React Testing Library).

Basically something is happening in some of your component code (or the code of your dependencies) after the test completes. You need to wait for those things to finish before your test completes (or your test is incomplete... no pun intended).

So in the case of @donaldpipowitch's demo, I added a single await wait() as the last line of the test and the error went away. Normally you'll want to have some kind of assertion for what actually happened there, but thanks to the fact that wait is wrapped in act we get the error to go away and if there is a problem, it will be more likely to surface.

Good luck!

@weyert
Copy link
Contributor

weyert commented Jan 22, 2020

Am I the only one that keeps forgetting that and get confused about the messaging? And then read a message like from @kentcdodds and think 'Yes of course!'

@kentcdodds
Copy link
Member

I don't think you're alone and I'm not sure how to help people understand this when they bump into it 😬

@cooervo
Copy link

cooervo commented Jan 27, 2020

I have bumped into this warning many times and now I finally solved it seems the problem was not await for the act function so no when I used it as in following code it works:

import { cleanup, fireEvent, render } from "@testing-library/react";
import "@testing-library/jest-dom/extend-expect";
import React from "react";
import RecaptchaSample from "../../../../../../src/pages/en/devtools/recaptcha-sample";
import { act } from "react-dom/test-utils";

jest.mock("../../../../../../src/utils/recaptcha-service", () => {
  return {
    __esModule: true,
    default: {
      getRecaptchaToken: async () => "TOKEN_EXAMPLE",
    },
  };
});

describe("RecaptchaSample", () => {
  afterEach(cleanup);

  it("it should get recaptcha", async () => {
    const recaptchaSample = render(<RecaptchaSample></RecaptchaSample>);
    const textArea = recaptchaSample.container.querySelector("textarea");

    const getRecaptchaButton = recaptchaSample.container.querySelector("button");
    expect(getRecaptchaButton).toHaveTextContent("Test Recaptcha");
    await act(async () => { .   /// notice await
      fireEvent.click(getRecaptchaButton);
    });
    expect(textArea.value).toBe("TOKEN_EXAMPLE");
  });
});

I believe this happens because my component has an async clickHandler:

const RecaptchaSample: NextPage<IProps> = () => {
  const [token, setToken] = useState("");

  const clickHandler = async () => {  // notice click handler is async
    const recaptchatoken = await RecaptchaService.getRecaptchaToken();
    setToken(recaptchatoken);
  };

  return (
    <div>
      <h1>Google Recaptcha Example</h1>
      <Button onClick={clickHandler}>Test Recaptcha</Button>
      <textarea css={styles.textAreaToken} value={token} readOnly={true}></textarea>
    </div>
  );
};

@sylwiasuwalska
Copy link

I have bumped into this warning many times and now I finally solved it seems the problem was not await for the act function so no when I used it as in following code it works:

import { cleanup, fireEvent, render } from "@testing-library/react";
import "@testing-library/jest-dom/extend-expect";
import React from "react";
import RecaptchaSample from "../../../../../../src/pages/en/devtools/recaptcha-sample";
import { act } from "react-dom/test-utils";

jest.mock("../../../../../../src/utils/recaptcha-service", () => {
  return {
    __esModule: true,
    default: {
      getRecaptchaToken: async () => "TOKEN_EXAMPLE",
    },
  };
});

describe("RecaptchaSample", () => {
  afterEach(cleanup);

  it("it should get recaptcha", async () => {
    const recaptchaSample = render(<RecaptchaSample></RecaptchaSample>);
    const textArea = recaptchaSample.container.querySelector("textarea");

    const getRecaptchaButton = recaptchaSample.container.querySelector("button");
    expect(getRecaptchaButton).toHaveTextContent("Test Recaptcha");
    await act(async () => { .   /// notice await
      fireEvent.click(getRecaptchaButton);
    });
    expect(textArea.value).toBe("TOKEN_EXAMPLE");
  });
});

I believe this happens because my component has an async clickHandler:

const RecaptchaSample: NextPage<IProps> = () => {
  const [token, setToken] = useState("");

  const clickHandler = async () => {  // notice click handler is async
    const recaptchatoken = await RecaptchaService.getRecaptchaToken();
    setToken(recaptchatoken);
  };

  return (
    <div>
      <h1>Google Recaptcha Example</h1>
      <Button onClick={clickHandler}>Test Recaptcha</Button>
      <textarea css={styles.textAreaToken} value={token} readOnly={true}></textarea>
    </div>
  );
};

Thanks for that. I also have async action on click and it was really hard to find this solution.

@lucksp
Copy link

lucksp commented Jul 17, 2020

act is not a promise, so you should not await it:

console.error node_modules/react-dom/cjs/react-dom-test-utils.development.js:87
Warning: Do not await the result of calling act(...) with sync logic, it is not a Promise.

@kentcdodds
Copy link
Member

Act is a promise that you can await if you pass it an async function.

However, in this case you shouldn't need to use act directly at all. 99% of the time, you shouldn't need to use act. All of the utilities exposed by @testing-library/react are automatically wrapped in act for you.

@liorpr
Copy link

liorpr commented Oct 26, 2020

got into a situation that I want to use document.querySelector for a complex query and not one the methods provider by react-testing-library, but I can't get rid of that act warning, any ideas? @kentcdodds

btw if I add a data-testid and then switch to findByTestId it works, just wondered how to remove the warning myself.

@nickserv
Copy link
Member

nickserv commented Nov 1, 2020

I've been able to fix this by either wrapping the document.querySelector call in waitFor (I feel like this only makes sense if it appears asyncronously) or by putting waitFor(() => {}) at the end of the test (similarly to what @kentcdodds suggested with await wait()).

In my case, it seems like my whole test can be synchronous, but React just complains about using Formik without act (unless I use waitFor). Is waitFor(() => {}) at the end still a good way to flush these warnings as with await wait()? If so, why is the callback required in waitFor? I would think it should be optional to encourage fixing the act warning. If not, what is the migration path for await wait()?

@cooervo
Copy link

cooervo commented Nov 1, 2020

For those having problem with act give this article a complete read it will clear things up:
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library

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