Prevent `act` warning on async hook

[mpeyper]

Currently, there is a warning about updating a hook outside of an act call when the update happens as the result of an async call (i.e. callback, promise, async/await, etc.)

There is an example of this in our own test suite which generates the warning when we run our tests.

Firstly I'd like this warning to go away. I've tried all the suggestions in @threepointone's examples repo to no avail. There has also been a rather lengthy discussion in the react-testing-library repo on the topic.

Secondly, if there is a way to remove the warning, the solution(s) should be documented in our docs to help others.

Finally, is there anything we could be doing/wrapping in our API so that users do not have to call act in their own code as often, similarly to the work that has already been done in react-testing-library?

I'm not 100% certain on whether there is anything we can actually do, so if you're more familiar with the act function and its uses, I'd love to get your input.

[panjiesw]

What a timing.

Experienced this just minutes ago.

It's fine and generates no warning if the async call is triggered by a previous update, because we can call 2 act, first to update state triggering the async call, and the second to call jest.runAllTimers().

But if the async call is immediately run in the first render, then it'll generate the warning, even with jest.runAllTimers() inside act after the render declration.

[mpeyper]

Sorry, I'm not sure I follow. As far as I can tell, any time the setter of a useState (and friends) call is called in a way that cannot be wrapped in an act call, the warning is present, regardless of which updated triggered the actual async call itself.

Or am I misunderstanding you?

[panjiesw]

It's one of technique in @threepointone's examples repo which involves polyfilling Promise using promise NPM package.

You can see in my fork of this repo. I changed asyncHook.test.js to my use case, add a test case illustrating this and skip other tests. This test case doesn't generate warning.

But there are times when async call need to be made immediately in useEffect like in your original test cases, there waitForNextUpdate() is useful but generates warning like described in this issue.

EDIT: looks like I'm misunderstanding something or my illustration's not valid after all

[panjiesw]

So it's indeed a miss in my test. I've updated my fork closer to how I did it.

We don't need waitForNextUpdate() if using combination of:

1. global promise polyfill
2. jest fake timers
3. and call jest.runAllTimers() inside act

Will wait for proper way without polyfill when act is more polished as mentioned in facebook/react#14769

[ab18556]

There is already a PR on react that should fix this facebook/react#14853
I wouldn't bother finding a workaround until this is released. It is only a warning after all ;)

[lvl99]

@ab18556 it is a warning, however it blocks CI builds if CI=true on create-react-app projects. CRA treats any warnings as errors and aborts the build, even if the tests passed.

[kvnwolf]

Same is happening for me when trying to test an asynchronous callback:

const handleIncrementAsync = useCallback(
  async () => {
    await sleep(2000);
    setCount(count + 1);
  },
  [count]
);

And the corresponding test case:

test("should handleIncrementAsync", async () => {
  const { result, waitForNextUpdate } = renderHook(() => useCounter());
  act(() => result.current.handleIncrementAsync());
  await waitForNextUpdate();
  expect(result.current.count).toBe(1);
});

With the same error message:

Warning: The callback passed to ReactTestUtils.act(...) function must not return anything.

It looks like you wrote ReactTestUtils.act(async () => ...), or returned a Promise from the callback passed to it. Putting asynchronous logic inside ReactTestUtils.act(...) is not supported.

It is also difficult to expect a promise thrown from an async hook, let's hope a solution lands soon :).

PD:

I made a code sandbox demonstrating this issue. Tests pass but it's still awkward to see this warning 🤷🏻‍♂️.

[ab18556]

@lvl99 I get your point, but since there is already a PR to address it, I don't feel adding a temporary workaround would really worth the effort.

According to this post, it should be released soon.
facebook/react#14769 (comment)

[lvl99]

@ab18556 no prob. I wasn't suggest it as a workaround, just giving some extra context on how it still blocks certain things, even if it is just a warning.

I'm currently dealing with a pipeline that refuses to build and are eagerly waiting for the react fix to land 😂

[mpeyper]

If the proper fix is on the way from the React team, then I agree it's not worth spending time on a short term work around.

CRA treats any warnings as errors and aborts the build, even if the tests passed.

@lvl99 would suppressing the output allow the build to pass?

function suppressWarnings(callback) {
  const consoleWarn = console.warn.bind(console)
  global.console.warn = () => {}
  callback().finally(() => {
    global.console.warn = consoleWarn
  })
}

test("should handleIncrementAsync", suppressWarnings(async () => {
  const { result, waitForNextUpdate } = renderHook(() => useCounter());
  act(() => result.current.handleIncrementAsync());
  await waitForNextUpdate();
  expect(result.current.count).toBe(1);
});

Note: untested

Not ideal, but might allow you to keep moving while you wait for a fix.

[threepointone]

we released a new alpha 16.9.0-alpha.0 that includes async act() to address these issues. longer docs are incoming, but it works as one would expect. example, taking @kevinwolfcr's example, rewritten with the new api

test("should handleIncrementAsync", async () => {
  const { result, waitForNextUpdate } = renderHook(() => useCounter());
  await act(async () => {
    result.current.handleIncrementAsync();
    await waitForNextUpdate();
  });
  expect(result.current.count).toBe(1);   
});

[mpeyper]

@threepointone

I have updated to the 16.9.0-alpha.0 version locally and updated and ran the tests and all the warnings go away, which is lovely.

I then made a change to waitForNextUpdate from

{
  // ...
  waitForNextUpdate: () => new Promise((resolve) => addResolver(resolve)),
  // ...
}

to

{
  // ...
  waitForNextUpdate: () => act(() => new Promise((resolve) => addResolver(resolve))),
  // ...
}

which allowed all the test cases in this library pass with no warning without having to make any changes, except on where multiple waitForNextUpdate are made in parallel (to test that they would all resolve on the next updated), where a warning states:

Warning: You seem to have overlapping act() calls, this is not supported. Be sure to await previous act() calls before making a new one. 

I like that the warning went away without having to change the tests, but I'm concerned that the act calls do not explicitly wrap the things making the updates (i.e. it only acts around the waiting part, not the whatever triggered the async update to occur), and that there is the potential to have overlapping act calls (not sure anyone would do this in the wild, or what the consequences there might be for doing it).

Happy to hear any suggestions or feedback.