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

Unhelpful warning for act for react-dom@16.8 #14769

Closed
ncphillips opened this issue Feb 6, 2019 · 128 comments
Closed

Unhelpful warning for act for react-dom@16.8 #14769

ncphillips opened this issue Feb 6, 2019 · 128 comments
Assignees

Comments

@ncphillips
Copy link

Do you want to request a feature or report a bug?

Feature/Improvement

What is the current behavior?

If there is test code that should be wrapped in act(...) then the current warning is given:

 console.error node_modules/react-dom/cjs/react-dom.development.js:506
    Warning: An update to null 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

When upgrading a large code base, this is basically useless.

What is the expected behavior?

Provide at least the test name if not the line number of code that triggered the warning.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

react@16.8.0
react-dom@16.8.0

@bvaughn
Copy link
Contributor

bvaughn commented Feb 6, 2019

Provide at least the test name if not the line number of code that triggered the warning.

console.error should display line numbers (and a call stack) which should include the test code that triggered this warning. Is this not happening?

@bvaughn
Copy link
Contributor

bvaughn commented Feb 6, 2019

When upgrading a large code base

Follow-up question: Does this imply that you have a large codebase making use of hooks?

@ncphillips
Copy link
Author

It is happening, but the error is in react-dom

image

Follow-up question: Does this imply that you have a large codebase making use of hooks?

Yup. At least I think the codebase is large, maybe just medium 🤷‍♂️. There's about 800 /.tsx?/ files. We (perhaps foolishly) have been using the alpha version in production for the last month and have been experimenting with hooks in a variety of places. It just so happens that one of those places is a widely used component that is being rendered in a lot of tests.

@bvaughn
Copy link
Contributor

bvaughn commented Feb 6, 2019

I wouldn't expect that. Do other console.error logs have their call stacks truncated as well?

To be clear, the error is logged from within react-dom – but the call stack should show the other frames that got to that point (e.g. where in react-dom was warningWithoutStack called) up to, and including, your individual test.

@threepointone
Copy link
Contributor

Could you also share your testing setup? Do you use enzyme/react-testing-library? How do you do async testing?

@gaearon
Copy link
Collaborator

gaearon commented Feb 6, 2019

"An update to null" looks weird. Can you try to create a reduced reproducing test case?

It is supposed to include the name of your component.

@gaearon
Copy link
Collaborator

gaearon commented Feb 6, 2019

Also, what test runner are you using? Can you create a minimal repro?

@ncphillips
Copy link
Author

We're using jest with react-testing-library.

I'll try to carve out some time later to get repro.

@threepointone
Copy link
Contributor

react testing library just released an update that integrates act by default with its render/fireEvent calls, so it should pass a bunch of your tests. Any remaining tests probably have explicit mutations, and you should wrap them with act() calls

@ncphillips
Copy link
Author

I did forget to update react-testing-library however upgrading to 5.5.1 did not fix the warnings. Maybe it did fix some, but not all. I don't have a count of how many warnings there were from running the tests.

Note: all our tests do pass, they just get those warnings.

@threepointone
Copy link
Contributor

Without act calls, it's likely your tests might not even be accurate. I updated a whole bunch of fb code for the fb codebase and it wasn't too bad - went through every warning, went to the test that the stack pointed to, and wrapped relevant calls with act() (and caught a couple of bugs in the process)

I'm curious why your stack is getting clipped.

@danielkcz
Copy link

danielkcz commented Feb 7, 2019

I think this issue is more about making that warning more useful. I've been fighting with same thing yesterday. For a reference have a look at travis output. It's also Jest + react-testing-library.

There is simply no means on figuring out what piece of code is causing that warning because Jest shows only a line where the console.error was produced. At least these are bundled per a test file for some initial clue, but it's not enough.

I think ideally this bulky warning should be there just once and then show only a more succinct variant for each occurrence that tells more about the location of it.

@ghengeveld
Copy link
Contributor

Having the same issue while trying to update the specs for useAsync to v16.8.1. The warning just refers to react-dom, not my own code.

schermafbeelding 2019-02-07 om 11 50 53

Besides this, I still have not found a way to actually fix the tests. Wrapping with act doesn't work. Using jest.useFakeTimers suggested here doesn't work either. Please provide decent documentation on how to write tests for custom hooks.

@ghengeveld
Copy link
Contributor

For the record, here's how I was testing the useAsync hook (simplified):

const Async = ({ children, ...props }) => {
  const renderProps = useAsync(props)
  return children(renderProps)
}

test("returns render props", async () => {
  const promiseFn = () => Promise.resolve("done")
  const component = <Async promiseFn={promiseFn}>{({ data }) => data || null}</Async>
  const { getByText } = render(component)
  await waitForElement(() => getByText("done"))
})

useAsync calls a whole bunch of different hooks underneith.

@bvaughn
Copy link
Contributor

bvaughn commented Feb 7, 2019

Please provide decent documentation on how to write tests for custom hooks.

I know it's a bit annoying at the moment, but please be patient with us. We're working on this (both the docs, and the underlying issue).

@ghengeveld
Copy link
Contributor

Good to know, and sorry for the harsh-sounding comment, you're doing an excellent job.

@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

Please provide decent documentation

If anything, being demanding in issues is a good way to discourage work on something. For example I've spent a lot of effort on this documentation (about a month of work), and comments like this make me not want to even open it anymore. Just something to keep in mind.

@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

@ncphillips We'd still appreciate a reproducing case to see how you got null in the warning message.

@ncphillips
Copy link
Author

Sorry @gaearon it's proving difficult to reproduce 🤔

It's hard to identify which tests are causing the warning, because they're all passing and since I don't have a warning count, I wouldn't even know if I fixed one.

I'm starting up a repo now to try and reproduce, but I don't have much time today.

@panjiesw
Copy link

panjiesw commented Feb 8, 2019

I created a simple repro for the warning message here https://github.com/panjiesw/act-warning. It uses jest (from CRA) and react-testing-library v5.5.3

In that repro, the test passed but the warning is still there and only showed component's name, without stack trace and line number

The component code:

import React, { useReducer, useEffect } from 'react';

function reducer(state, action) {
  switch (action.type) {
    case 'value':
      return {
        ...state,
        value: action.payload,
      };
    case 'isBar':
      return {
        ...state,
        isBar: action.payload,
      };
    default:
      return state;
  }
}

const Input = () => {
  const [state, dispatch] = useReducer(reducer, { value: '', isBar: 'no' });

  useEffect(() => {
    let mounted = true;
    Promise.resolve().then(() => {
      if (mounted) {
        dispatch({
          type: 'isBar',
          payload: state.value === 'bar' ? 'yes' : 'no',
        });
      }
    });
    return () => {
      mounted = false;
    };
  }, [state.value]);

  const onChange = e => {
    dispatch({ type: 'value', payload: e.target.value });
  };

  return (
    <div>
      <input data-testid="foo" value={state.value} onChange={onChange} />
      <div data-testid="check">isBar: {state.isBar}</div>
    </div>
  );
};

export default Input;

The test:

import React from 'react';
import { render, fireEvent, cleanup } from 'react-testing-library';
import Input from './Input';

afterEach(cleanup);

jest.useFakeTimers();

test('Input using hooks', done => {
  const w = render(<Input />);

  fireEvent.change(w.getByTestId('foo'), {
    target: { value: 'bar' },
  });

  process.nextTick(() => {
    expect(w.getByTestId('foo').value).toEqual('bar');
    expect(w.getByTestId('check').textContent).toEqual('isBar: yes');
    done();
  });
});

@threepointone threepointone self-assigned this Feb 8, 2019
@hisapy
Copy link

hisapy commented Feb 8, 2019

Hi devs,

In my case, I'm testing a simple component that useState with mount from enzyme.

Notice that I'm not testing the hooks. I'm just mounting the component and simulating some onChange and onClick and then asserting what props changed.

Everything passes except that I get the warning:

console.error ../../node_modules/react-dom/cjs/react-dom.development.js:506
    Warning: An update to _default 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 the docs it says:

To reduce the boilerplate, we recommend using react-testing-library which is designed to encourage writing tests that use your components as the end users do.

What does it means for enzyme users? I have a ton of tests that are working but a ton of these warnings as well ... Perhaps, is there a way to silence them?

@ghengeveld
Copy link
Contributor

It's not just Enzyme, you'd have the same issue with react-testing-library right now. The team is aware of the issue and working on improving the situation, but having just launched a revolutionary new API means they are probably scrambling to fix a whole lot of other issues as well. It's frustrating if you want to keep up to date with the latest and greatest, but that's how it is. For now I'm just going to disable the affected tests and fly with it.

@swapkats
Copy link

How could I disable this warning. I use jest and enzyme. I understand the implications but would really like to suppress this warning as it causes unnecessary scrolling and sort of throws me off my workflow.

@hisapy
Copy link

hisapy commented Feb 11, 2019

I added this to my test files

// Silence until enzyme fixed to use ReactTestUtils.act()
// If not silenced we get the following warning:
// console.error../../ node_modules / react - dom / cjs / react - dom.development.js: 506
// Warning: An update to _default 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

const originalError = console.error;

beforeAll(() => {
  console.error = jest.fn();
});

afterAll(() => {
  console.error = originalError;
});

describe("onSubmit login", () => {
  // tests here
})

If you want to suppress the warnings globally instead of file by file, you might want to try the above in a jest setup file

@gaearon
Copy link
Collaborator

gaearon commented Feb 11, 2019

Sorry folks — again, we’ll be looking into different scenarios soon and adjust the heuristics.

If you have a test suite full of warnings immediately after a new API that has previously been called an alpha was released then I’m sure you can tolerate living with warnings in tests for a few more days. :-)

but would really like to suppress this warning

We do not recommend to silence it hoping that Enzyme will fix them for you.

In some cases, you will need to actually wrap your code into act() sections. Again, we’ll need to adjust the heuristics and provide more guidance. But I would avoid writing large test suites around APIs while they are in alpha versions if you can’t tolerate some warnings or aren’t sure how to suppress them.

@flucivja
Copy link

flucivja commented Feb 11, 2019

Hello, the problem I have is that even I wrap code in act I receive the warning and I am not sure how to wrap code to avoid it (I tried several combinations without luck).

Here is gist I created to reproduce issue: https://gist.github.com/flucivja/36a324f183c212d0e1cf449631e725a3

At the first I thought it is enzyme issue but then I did same without it and warning was still there.

@ajur58
Copy link

ajur58 commented Feb 11, 2019

Not sure if this is the best place for feedback on act(), sorry if it's off-topic.

While encountering the issue with the warnings, heading to the docs & ending up in this thread, I wondered about two things:

  • Do we need to use act() only on components using hooks?
  • Should we consider the code running after act() as sync? I get what act does, but I don't get how the next line (the assertion) waits for its execution.

@gaearon
Copy link
Collaborator

gaearon commented Jul 30, 2019

We just published 0.0.0-db3ae32b8 of all packages. It is likely going to be a release candidate for 16.9.0 so you can give it a try if you'd like before it gets promoted to stable.

@threepointone
Copy link
Contributor

16.9 got released, including async act, and updated documentation https://reactjs.org/blog/2019/08/08/react-v16.9.0.html Closing this, cheers.

@mharidy
Copy link

mharidy commented Aug 14, 2019

upgrading react and react-dom to 16.9 doesn't fix the problem, the only change console.error node_modules/react-dom/cjs/react-dom.development.js:545 insted of
console.error node_modules/react-dom/cjs/react-dom.development.js:506

@danielkcz
Copy link

@mharidy See #16348 where I tried to improve that and now it continues over at jestjs/jest#8819

@ernestohegi
Copy link

ernestohegi commented Aug 17, 2019

https://reactjs.org/blog/2019/08/08/react-v16.9.0.html#async-act-for-testing this solved all my problems when using custom async hooks. No more act warnings and tests are passing! Thanks a lot!

@pavanshinde47
Copy link

Can you please share an example of your test?

@ernestohegi
Copy link

Something along these lines:

  it('should render categories', async () => {
    let wrapper;

    await act(async () => {
      wrapper = mount(<HeaderCategories />);
    });

    // Triggers update.
    wrapper.setProps();

    expect(wrapper.find('HeaderCategory')).toHaveLength(2);
  });

@gre
Copy link
Contributor

gre commented Sep 13, 2019

is there a way to actively disable this log feature with React?

It is not helpful for case like when you run tests from inside a React app. I've wrote a "jest-like" runner inside React Native that run my tests. the test does not involve React testing at all! but the fact the tests are run FROM react creates these crazy logs.

as a "test runner implementer" how do i disable this log to happen.

Thanks.

Capture d’écran 2019-09-13 à 14 30 03

@jktravis
Copy link

I had a couple tests that would do this, and had a really hard time finding the problem. But for me, I was using react-testing-library, I just added an extra await wait() at the bottom of each test that was giving me the problem.

Maybe you could implement an inherit wait for every test?

@threepointone
Copy link
Contributor

There's no way to disable the warning in your scenario, you could try patching console.error.

I had a couple tests that would do this, and had a really hard time finding the problem. But for me, I was using react-testing-library, I just added an extra await wait() at the bottom of each test that was giving me the problem.

Maybe you could implement an inherit wait for every test?

I would not recommend this. @jktravis I'd recommend instead fixing the test and wrapping the section generating the async update with await act(async () => ...)

@pelotom
Copy link

pelotom commented Sep 13, 2019

How can fireEvent.click return a promise if it doesn't know when all the changes it kicked off have actually completed? Clicking a button could trigger a fetch which asynchronously updates your component some time later; there's no way for fireEvent.click to know that.

@jednano
Copy link

jednano commented Dec 10, 2019

I'm still getting this error with react-scripts@3.3.0 (recently published) and React v16.12.0. Specifically, I'm getting it with React Testing Library, whenever I call a useReducer-generated dispatch within a useEffect.

See this RTL issue for more context.

@rahulcyadav
Copy link

@threepointone could you help fix this test
https://codesandbox.io/s/jest-enzyme-re41k

@danielkcz
Copy link

danielkcz commented Jan 4, 2020

@rahulcyadav You should probably ask over at Jest repo. Error is popping from Jest code. Besides, there is no problem with "act" warning. This isn't general "fix my tests" issue :)

Personally, I would recommend you to leave Enzyme and use react-testing-library instead. There is a couple of pain points with Enzyme that are not worth it.

@gaearon
Copy link
Collaborator

gaearon commented Apr 19, 2020

Jest 25.4.0 includes a JS stack for console.error so this should improve things. They're asking for feedback! jestjs/jest#8819 (comment)

@Siddharth77
Copy link

Siddharth77 commented May 6, 2020

My sincere THANKS to @ernestohegi for guiding with his solution and I did bit refactoring of the same and the thing which has resolved error for me is as below :

`/* eslint-disable react/prop-types */
import React from 'react';
import ReactDOM from 'react-dom';
import { act } from 'react-dom/test-utils';
import { createMount } from '@material-ui/core/test-utils';
import xComponent from './xComponent';

let mount = createMount();

beforeEach(() => {
mount = createMount({ strict: true });
});

afterEach(() => {
mount.cleanUp();
});

it('renders without crashing', async () => {
const div = document.createElement('div');
await act(async () => {
ReactDOM.render(, div);
});
ReactDOM.unmountComponentAtNode(div);
});`

@AncientSwordRage
Copy link

I'm still seeing this despite having react 16.13.1 and jest 24.9.0, any idea on how to diagnose?

mikecoll added a commit to mikecoll/react-gesture that referenced this issue Dec 12, 2020
admin-turris pushed a commit to turris-cz/reforis that referenced this issue Nov 25, 2021
…of react-test-renderer.

For async act support.
See facebook/react#14769
It should be updated to stable version in future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests