[5.5.0] Getting warning about "act" anyway
danielkcz opened this issue · 128 comments
Thanks for the fast work Kent, but seems something is still off. I've updated to 5.5.0 and tests are still generating same act warnings. Am I missing something here? Do we need to actually use act inside the tests?
https://travis-ci.org/mobxjs/mobx-react-lite/builds/489613981
It's curious that I've actually removed a bunch of flushEffects calls which were necessary before and tests are passing just fine. Clearly act helped with that. And yet those warnings shouldn't be there. 🤔
I can't see your tests, but I'm guessing you're triggering some changes 'manually' (ie - without fireEvent). those calls should be wrapped with act() too.
Funny enough there is not a single fireEvent. It's a MobX library which is about reactions :) Ultimately it all boils down to this piece of code which is just useState incrementing integer for each update.
I guess I will wait when you can have a look at those test files because I am not sure how to handle this reality. For a reference, a test file like this is causing the warning.
https://github.com/mobxjs/mobx-react-lite/blob/master/test/ObserverComponent.test.tsx
Just got bitten by this, upgraded to 5.5.0, remove flushEffect and the warning is still there. I use fireEvent from react-testing-library. Double check the js file, and it's indeed using wrapped fireEvent of dom-testing-library inside actCompat callback.
EDIT to include the test:
import { render, fireEvent } from 'react-testing-library';
// Other imports and tests
it('sets and reacts to data', () => {
const w = render(
<FieldWrapper>
<FieldInput id="foo" />
</FieldWrapper>,
);
fireEvent.change(w.getByTestId('foo'), {
target: { value: 'bar' },
});
expect(w.getByTestId('foo')).toHaveAttribute('value', 'bar');
});A codesandbox would be very helpful.
yeah I can't help more without a working example, sorry, basically shooting blind.
Yeah, that change event basically also trigger bunch of effects (async processing) behind the scene, which when resolved updates other state.
I tried to simulate it in this sandbox https://codesandbox.io/s/rjnwr62v5o
The component is in src/input.js and the test is in src/__tests__/input.js. As you can see there, the test passed, but the warning also appeared in the console.
Gotcha, so what @threepointone said is true:
I'm guessing you're triggering some changes 'manually'
So it's working as designed.
I'm honestly not sure how to account for situations like this though. What do you think Sunil?
I am kinda unclear what "manual update" means. Is invoking a setter from the useState considered a manual update? Isn't that going to be in nearly every component? That would make writing tests somewhat more annoying.
Heading home so I’ll get to this in a bit, but tldr - jest.useFakeTimers/jest.runAllTimers should help. Back in an hour.
Regarding the call stack.
Any time you call a state updater function like this.setState, or dispatch or setState, if that's not happening within the call stack of a batch update in react then you'll get this warning because it basically means you're test is doing something that wouldn't happen in the browser (this is where things get fuzzy for me, I'm not sure why that is).
Anyway, the act utility puts your callback within a batch update. But because you're calling a state updater in an async callback (via Promise.resolve().then), it's running outside a batch update and therefore you get the warning.
We need to find a good way to either work around this and/or document this.
Note: This has nothing to do with react-testing-library or even hooks and everything to do with new warnings that were introduced in React@16.8.0 to make your tests better resemble reality. As I said, I'm not sure what the difference is. Maybe Sunil can help explain it.
I've prepared another code sandbox reproducing the issue.
The warning comes from the first test. The second test is failing and the last one is only one working properly, but super ugly and I really hope there will be a better way.
@FredyC I got it working for the first test without warning:
it("works, but shows warning", () => {
const state = mobx.observable({ value: 5 });
const { container } = render(<TestComponent state={state} />);
expect(container.textContent).toBe("5");
// This updates state, so should be wrapped in act
act(() => {
state.value = 10;
})
expect(container.textContent).toBe("10");
});AFAIK, this is consistent by the react-dom/test-utils docs, it's just that the example in the docs is firing event, but basically the same as updating state.
^ this is correct. I’ll edit the docs to be clearer to say that you need to wrap anything that causes updates to be wrapped.
Ah, now that is starting to make sense, but it's ugly nonetheless considering that this was working just fine pre-16.8. I was kinda hoping that with the official release there will be an improvement to testing experience, not that it will be even worse 😢
Hmmm... I'm still not certain how to solve for cases like this:
// this is similar to __local_tests__/async-with-mock.js
// except this one uses a form of dependency injection because
// jest.mock is not available in codesandbox
import React from 'react'
import axios from 'axios'
import {render, fireEvent, waitForElement, cleanup} from 'react-testing-library'
import 'jest-dom/extend-expect'
afterEach(cleanup)
function useFetch({url, axios = axios}) {
const [state, setState] = React.useState({})
const fetch = async () => {
const response = await axios.get(url)
setState(s => ({...s, data: response.data}))
}
const ref = React.useRef()
React.useEffect(
() => {
if (ref.current) {
ref.current = true
} else {
fetch()
}
},
[url],
)
return {state, fetch}
}
function Fetch({url, axios}) {
const {state, fetch} = useFetch({url, axios})
const {data} = state
return (
<div>
<button onClick={fetch}>Fetch</button>
{data ? <span data-testid="greeting">{data.greeting}</span> : null}
</div>
)
}
test('Fetch makes an API call and displays the greeting', async () => {
const fakeAxios = {
get: jest.fn(() => Promise.resolve({data: {greeting: 'hello there'}})),
}
const url = 'https://example.com/get-hello-there'
const {getByText, getByTestId} = render(<Fetch url={url} axios={fakeAxios} />)
fireEvent.click(getByText(/fetch/i))
const greetingNode = await waitForElement(() => getByTestId('greeting'))
expect(fakeAxios.get).toHaveBeenCalledTimes(2) // React runs the render twice in dev mode
expect(fakeAxios.get).toHaveBeenCalledWith(url)
expect(greetingNode).toHaveTextContent('hello there')
})I get the intentions though, by avoiding the warning we can be sure that the state update is intended. Same with mobx under strict mode where observable mutation must be used within action, runInAction and its family.
But then the strict mode is configurable in mobx. So, maybe make act requirement somehow configurable? I don't know if it'll make things easier or more complicated
And another use case where it will be even more annoying.
let oldTodo: any // nooooo, I am too sexy for this :(
act(() => {
oldTodo = store.todos.pop()
})
expect(getDNode(oldTodo, "title").observers.size).toBe(0)And in the TypeScript world this means falling back to any escape hatch because it will complain seriously about such use.
I get the intentions though, by avoiding the warning we can be sure that the state update is intended
That might be true, but those warnings don't really give a useful hint where in the code is the problem happening. It would need to be improved for sure.
That might be true, but those warnings don't really give a useful hint where in the code is the problem happening.
are you not seeing a stack trace under your warning?
just got home, working on your fetch example @kentcdodds
I was kinda hoping that with the official release there will be an improvement to testing experience, not that it will be even worse
Consider that this will make your tests more reliable. This is apparent most when you fire multiple events in a row, effects cascade (or even just get triggered). I wouldn't be surprised if fixing all the warnings leads to discovering actual bugs in your code. That was my experience when doing the same for fb.
are you not seeing a stack trace under your warning?
Not really, you can see the output in code sandboxes presented here or in my linked Travis build. Jest is only showing the line where console.error got called: node_modules/react-dom/cjs/react-dom.development.js:506. Should I open react issue about improving so it's actually helpful? :)
A good thing is I've managed to fix mobx-react-lite tests and it working just fine. I guess in time it will become a habit.
I'm still seeing the act warning with some of my tests as well. Unfortunately, I cannot share the source, however, I can give as much context as possible. I have a useEffect hook that makes an axios request and stores the result in state (using setState), which seems to be causing the issue.
Here is what one of my tests look like for a context provider:
test('Should select first bot id and pass to provider', async () => {
axios.get = jest.fn().mockResolvedValueOnce({
data: {
data: myData
}
})
const { getByText } = render(
<MyProvider>
<MyContext.Consumer>
{value => <span>Received: {value.myValue}</span>}
</MyContext.Consumer>
</MyProvider>
)
const receivedElement = await waitForElement(
() => getByText(/^Received:/).textContent
)
expect(receivedElement).toBe(`Received: my test text`)
})
The tests succeeds, however I'm seeing the console warn, any suggestions / ideas as to why this is happening? Please let me know if I can provide any further clarification.
same problem as the one described in the last post of Kent
test("renders data", async () => {
const spy = jest
.spyOn(Fetch, "doFetch")
.mockReturnValue(Promise.resolve(createResponse(items)));
const { getAllByTestId } = render(<Items />);
const elements = await waitForElement(() => getAllByTestId(dataTestIdItem));
expect(elements.length).toBe(items.length);
spy.mockRestore();
});It should working:
jest.useFakeTimers();
const spy = jest
.spyOn(Fetch, "doFetch")
.mockReturnValue(Promise.resolve(createResponse(items)));
const { getAllByTestId } = render(<Items />);
act(() => {
jest.runAllTimers();
})
const elements = await waitForElement(() => getAllByTestId(dataTestIdItem));
expect(elements.length).toBe(items.length);
spy.mockRestore();
});
I've got a problem with fetch and catch errors.
So I'm currently updating the tests for useAsync to work with 16.8, hitting this exact problem.
I tried using {act} from react-dom/test-utils, as well as the useFakeTimers trick mentioned above. So far I wasn't successful. Any pointers would be great.
test("returns render props", async () => {
jest.useFakeTimers()
const promiseFn = () => new Promise(resolve => setTimeout(resolve, 0, "done"))
const component = <Async promiseFn={promiseFn}>{({ data }) => data || null}</Async>
const { getByText } = render(component)
act(() => {
jest.runAllTimers()
})
await waitForElement(() => getByText("done"))
})Wrapping stuff with act is really annoying, as it doesn't work well with async/await.
just a heads up, I've been wanting to answer this in detail, but just swamped with a bunch of other things with the team. we're aware of the problems, promise. I'll ping here when I have something to share.
Good to know you're aware. I trust you'll come up with a good solution. Keep up the good work!
Also running into the same issue. I'm curious if you're planning on incorporating act into RTL's functionality for things like fireEvent and render, or if we should go ahead and plan on wrapping all of our state updating actions in tests in an act?
@brandon-lile Imo it should become a good practice to wrap any state changes to act. It could be rather confusing if some of the state changes would be covered automatically and some not. Personally, I would keep it for an initial render only.
@brandon-lile act() has already been added to RTL with #280 . I think this issue still remains and is not caused by RTL, rather an issue on the react side of things.
Exactly what I needed to know. Thank you!
I'm working on a details post/proper fix, but to alleviate you all, I wrote some quick details here - facebook/react#14769 (comment)
@threepointone I'm not sure the act should act that way. I don't have any fire event in mine. All I had was an API request and a state change in useEffects after the request resolves.
https://stackoverflow.com/questions/54748942/why-does-react-hook-throw-the-act-error-when-used-with-fetch-api/54749499#54749499
@Gpx yeah, except that an event triggers the state changes 🤔
Here, it's useEffects since it happens after the component has mounted and the fetch API has resolved. But wrapping the component to be rendered in act doesn't solve this.
This issue almost broke my excitement for hooks: we now have to introduce implementation details in our tests (act()) and it sounds weird to me now that I'm used to write clean tests wit RTL.
@threepointone documentation brings a lot of nice tricks: https://github.com/threepointone/react-act-examples but they still are introducing implementation details.
The following pattern was working seemlessly with componentDidMount():
and having to rewrite my tests makes me feel bad.
I definitely understand why the warnings are fired about using act in this case but I hope we can find a solution in the future that could be run under the hood by RTL so our tests don't have to be updated.
@ValentinH I wouldn't really call that implementation detail. Welcome to the async world :) Since normally React won't do a bunch of stuff synchronously (are more is to come), then obviously you need some test utility to switch it back to synchronous way otherwise those tests would be super complicated.
If you recall (maybe not), before the act came to light, there was ugly hacks like double re-rendering or flushEffects. These were indeed testing "implementation details". Now instead of thinking if there is some side effect or whatnot, you simply need to mark the location in a test that does some sort of "interACTion" with a user.
I think that act is a nice backdoor around the problem, it just needs a little bit polish a figure out some scenarios.
@FredyC I definitely understand your point about async and I do agree that act is much better than the previous solutions.
However, it's still an implementation detail in my opinion. I might be wrong but I think you could write a component having exactly the same behavior with a data fetching on load:
- in a class using
componentDidMount() - in a function using
useEffect()(or evenuseLayoutEffect()to be really comparable)
In the end, you would have to write a slightly different test due to act().
I definitely do not want to be the guy criticizing everything, I'm just sharing a concern 😀
I’m working on editing the doc, in particular I’m looking to show how it’s useful for classes too. In the middle of a cold tho, so it might take a couple of days.
@ValentinH It's all about tradeoffs. With classes, you have only a single componentDidMount and you kinda have to pile up everything in there (and mix that with other methods too). The Hooks give you the ability to separate your side effect concerns in much cleaner and approachable way.
Btw, cDM to initiate data fetching was always kind of a hack. With Suspense coming later this year (hopefully) you won't even need useEffect. If you still haven't seen the video from React Conf, then go ahead: https://youtu.be/ByBPyMBTzM0?t=1340
Note: It's a link to a specific time that's about this topic, but I really do recommend to watch the whole thing.
No worries, I've watched this awesome video many times and I understand why Hooks are a huge improvements for React. 😍
Raising a concern about act doesn't mean I'm against hooks and I understand there might be tradeoffs.
My goal is simply to understand if something could be improved to avoid this and if I could help.
My main fear is having to teach how to test hooks to my colleagues while I was the one arguing to stop using Enzyme because of implementation details (even without this, I love RTL btw).
One concern I have is why waitForElement() doesn't work if you don't call act(). It seems like the JSDOM env doesn't match the browser experience, which would eventually evaluate effects. I would like it be possible to use something like the proposed async queries testing-library/dom-testing-library#203 for 90% of the use cases where you don't care about exact batching.
To put it another way - we are already asynchronously awaiting the effects of user interactions by retrying queries until a condition is matched. Tests written assuming an async result don't break due to batching or scheduling unless the timeout is exceeded.
Edit: seems like it waitForElement works with useEffect now, at least with jsdom 13?
I'll also note that Sunil has some changes in mind which will make it so you rarely have to use act at all (because react-testing-library will do it for you) even in the async case.
This would be awesome!
Taking a closer look at @threepointone's document, I feel like a lot of the examples, if not all, were already handled by existing dom-testing-library patterns.
Edit: except the batched click handler
For example:
Even with classes the only way to test an async action in cDM was this:
const {getByText} = render(<Comp />)
// asserting here always failed even with classes + cDM
// use waitForElement to retry instead:
await waitForElement(() => getByText('whatever'))And responding to a user interaction looked like this:
const {getByText} = render(<Comp />)
fireEvent.click(getByText('a button'))
await waitForElement(() => getByText('this element appears later'))Also - doesn't batching promise updates gloss over some possible states that would be perceptible to the user? For example, an async call in cDM might be mocked by a Promise that resolves in 1 tick in test world, but 0.5s in real life. Squashing those updates seems wrong if there is no way to inspect the intermediate state.
We can now see the problem. We run our test at a point in time when react hasn't even finished updating the UI. You could hack around this:
- by using useLayoutEffect instead of useEffect: while this would pass the test, we've changed product behaviour for no good reason, and likely to its detriment.
- by waiting for some time, like 100ms or so: this is pretty ick, and might not even work depending on your setup.
This section is missing the way waitForElement actually works - use MutationObserver to re-run when the DOM changes.
Put my questions into this repo: https://github.com/alexkrolick/react-testing-examples
I know this is not a solution, but if someone wants to temporarily "mask" the warnings, he/she can try mocking the actual implementation of the useState and useReducer hooks, so that all the state updates are always wrapped in an act() .
That means mocking React with something like that:
// under __mocks__/react.js
const realModule = jest.requireActual('react');
module.exports = {
...realModule,
useState: initialState => {
const [state, setState] = realModule.useState(initialState);
return [
state,
value => {
require('react-dom/test-utils').act(() => {
setState(value);
});
},
];
},
useReducer: (reducer, initialState) => {
const [state, dispatch] = realModule.useReducer(reducer, initialState);
return [
state,
action => {
require('react-dom/test-utils').act(() => {
dispatch(action);
});
},
];
},
};Again I want to stress that you are not solving the issue, but masking the warnings until a solution comes from the React team. I'm only leaving it here in case someone want to temporarily use it :)
W would prefer to have complementary wrapper so you wrap your assertion rather modification.
Instead of guessing what may cause update.
it('some test case', ()=>{
// do anything with your test case
React.whenCoherent(()=>{
expect(....)
})
})Pretty specific question:
We use babel to cross-compile async/await to generators and it seems like we're having trouble getting it to play nicely with act and multiple layers of async/await and a setState (from useState) behind it.
We keep getting a warning that we should wrap our triggering of the promise resolution with act but it seems like the regenerators keep breaking it out. jest.runAllTimers also doesn't seem to help. Any ideas?
We would really like to avoid mocking each layer of async/await separately because that's highly private to the implementation
Our latest attempt looks like this:
test('fetches something when the button is pressed', () => {
jest.useFakeTimers();
axiosMock.get.mockReturnValue({
then(callback) {
act(() => {
jest.runAllTimers();
callback({ data: {} });
jest.runAllTimers();
});
}
});
const { queryByText } = render(<Demo someProp="foo" initialCatUrl="some-url" />);
const button = queryByText('Fetch something!');
act(() => {
fireEvent.click(button);
});
expect(axiosMock.get).toBeCalledTimes(1);
});hello all, just wanted to throw my use-case onto the pile:
i created a very naive useMiddlewareReducer hook:
import {useReducer} from 'react';
export default function useMiddlewareReducer(reducer, initial, middleware = []) {
const [state, rawDispatch] = useReducer(reducer, initial);
const dispatch = (action) => {
middleware.forEach(({type, fn}) => action.type === type && fn(action, rawDispatch));
rawDispatch(action);
};
return [state, dispatch];
}
and then i used it in a component kind of like so:
function Component({stuff}) {
const [state, dispatch] = useMiddlewareReducer(someReducer, someInitialState, [
{
type: 'begin',
fn: (action, dispatch) => {
try {
const result = await service.doAsyncThing(stuff);
dispatch({type: 'complete', result});
} catch (error) {
dispatch({type: 'error', error});
}
}
}
]);
return <SomePresentationalThing state={state} />;
}
everything has been going swimmingly up until the point that i try to make service.doAsyncThing throw an error. React DOM warns me about me not using act (though i am wrapping my call that causes the method to throw in act). The test still actually works properly, which I can verify by logging out the data I'm expecting etc. If I wrap the inner dispatch call with act, I don't get the error. I'm assuming that when I try to write the same test but for a successful resolution, the 'complete' dispatch will also cause React DOM to warn me about not using act.
I also have this issue as soon as I use waitForDomChange().
I'm sorry if Is not the good place to put this, but in the same subject I got a case, and I'm not sure if i missunderstand something with that :
https://codesandbox.io/s/22oloo7zn
I got callbacks in a useEffect, and in that case, i dont know how can I Wait for the end of the useEffect hook.
I could reproduce this ine the codesandbox right here, so i think, I missunderstand solthing, right ?
Thanks.
@oyouf Your code does not make sense. You are not passing promise and test anywhere in your test file :) I don't recommend having a factory function around your component, rather pass those data in the props directly. And since you are passing the promise in, it's easy to wait for it in the test. And don't wrap render in the act, the RTL does that for you already.
@FredyC Thanks for your reply =). I changed act, by wait and it works.
You said you don't recommend factory function, but i want to make dependency injection...
@oyouf Problem with such a pattern is that you will get a new component object which means to React to remount whole thing. If that's not an issue, go ahead, but I would really recommend using React Context instead for a "dependency injection".
Hey @kentcdodds @threepointone, I'm having a bit of trouble making RTL and hooks work with async/await + useEffect + useState.
I've created a small codesandbox example with the problem: https://codesandbox.io/s/8n34on95w9
Am I missing something?
@ivoreis You are missing this :) facebook/react#14853
@ivoreis you should still be able to test this code, but you are not awaiting for any changes. The promise is resolved after your expect() so at the moment it is runned, the value is still 2.
To be able to await the update before your expect() is runned, you could test your hook with https://github.com/mpeyper/react-hooks-testing-library
Using the waitForNextUpdate() method this library provides, you should be able to test your code.
It will issue a warning about the fact you are not using Act() tho. As @FredyC said, to remove this warning, you will have to wait until facebook/react#14853 is released.
Alright everybody, the PR to add async act was merged (thanks @threepointone!), so once the alpha is released can everyone check if they are still seeing these warnings?
We'll eventually want to wrap our wait utilities in it so people don't have to do it manually 😀
https://github.com/facebook/react/releases/tag/v16.9.0-alpha.0 :D
docs incoming. @kentcdodds, happy to help you integrate this, lemme know.
I'm also happy to use this issue as a discussion ground for whether the async version worked for you folks. I took a quick spin at updating my readme/examples in https://github.com/threepointone/react-act-examples (still unfinished, but should be helpful for this group now), and will spend today working through the failures I see in this thread (and others in the react repo). Let me know!
@magrytos, could you try the alpha to see if it removes the warning you were seeing yesterday?
I've tested my code with react@16.9.0-alpha.0 and I'm still seeing the warning @ValentinH.
My code looks something like:
const [needsDataFetch, setNeedsDataFetch] = useState(true)
const [data, setData] = useState([])
const fetchData = async () => {
const response = await axios.get(url)
setData(response)
setNeedsDataFetch(false)
}
useEffect(() => {
if (needsDataFetch) {
fetchData()
}
}, [needsDataFetch])Then I'm testing it with react-testing-library render method and the warning pops up:
Warning: An update to RecommendedArticles inside a test was not wrapped in act(...).
It won't work magically by updating React version in a project. The RTL has to be changed to use the await act.
It essentially means that the whole render needs to be async which is going to be ugly breaking change AFAIK. All our tests would need to become async to await render.
Same story for fireEvent, we will need to be awaiting it.
Sure, ultimately it's definitely the safest approach, but I would argue that most tests don't need it really. Could we have renderAsync and fireEventAsync perhaps?
render is still sync. act works sync if you pass it a sync function, and async if you pass it an async function. you don't have to change your existing tests.
@magrytos can you show me your actual test? best if you can show it on codesandbox.
@FredyC to be clear - this isn't true ->
It essentially means that the whole render needs to be async which is going to be ugly breaking change AFAIK. All our tests would need to become async to await render.
Same story for fireEvent, we will need to be awaiting it.
Sure, ultimately it's definitely the safest approach, but I would argue that most tests don't need it really. Could we have renderAsync and fireEventAsync perhaps?
What's most likely, is the async helpers will be wrapped with async act, and the sync ones will stay as they are.
I am confused. So you are saying that even though the render function in RTL is using sync version, it should work with async code?
@magrytos obviously does have a async code, so you are saying it should work without modifying RTL?
What's most likely, is the async helpers will be wrapped with async act, and the sync ones will stay as they are.
Isn't that what I was suggesting with renderAsync and fireEventAsync?
This is more easily explainable with actual tests, or if you read my doc. you'll note that their are sync sections, and async sections. for rtl, this means async helpers like waitForElement, will trigger the async flow, while sync helpers like render and fireEvent will still use the sync version. say you want to capture async effects on a sync action (like fireEvent), you'd just write a manual async act() around it yourself. (or you could make a fireEventAsync helper as well, but my point is you don't have to change existing tests/helpers)
Sorry, my bad. I didn't wrap my test in act previously. Now everything works fine, here is my test:
it('test my component', async () => {
const axiosSpy = jest.spyOn(axios, 'get').mockResolvedValue('... some data')
await act(async () => {
const { getByText } = render( <MyComponent /> )
await waitForElement(() => getByText('this text should be present'))
})
expect(axiosSpy).toHaveBeenCalledWith('... some params')
})Just to be sure, once the async act() is used in RTL, will we still need to use it manually in our tests like @magrytos above?
I am still struggling to get this to work. I updated react to 16.9.0-alpha.0 and am using rtl 6.0.3.
My test:
it('renders an api status message if there is one', async () => {
mockAxios.get.mockImplementation((args) => {
if (args === 'myblog.com/rss.xml') {
return Promise.resolve(blogResult);
}
if (args === 'https://lgbfddhklcdg.statuspage.io/api/v2/status.json') {
return Promise.resolve(statusBadMessage);
}
});
const { container, getByText, asFragment, queryByText } = render(
<AnnouncementBarWithRouter />
);
await wait(() => getByText('Test Header'));
// check API calls
expect(mockAxios.get).toHaveBeenCalledTimes(2);
expect(mockAxios.get).toHaveBeenCalledWith('https://lgbfddhklcdg.statuspage.io/api/v2/status.json');
expect(mockAxios.get).toHaveBeenCalledWith('myblog.com/rss.xml');
// check what was rendered
expect(getByText('Test Header')).toBeInTheDocument();
expect(asFragment).toMatchSnapshot();
// make sure blog post wasnt rendered
const BlogPost = queryByText('Platform Update Roundup 2018');
expect(BlogPost).toBeNull();
});
This passes but I still getting two warnings that are both
Warning: An update to AnnouncementBar 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 AnnouncementBar (at announcement-bar.test.js:168)
in Router (created by BrowserRouter)
in BrowserRouter (at announcement-bar.test.js:167)
in AnnouncementBarWithRouter (at announcement-bar.test.js:218)
I am assuming it's because the promises called from the data fetching resolve after the initial render (since it produced two warnings), but when I wrap the wait in act,
await act(async () => {
await wait(() => getByText('Test Header'));
});
I get the following errors:
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.
console.error node_modules/jest-prop-type-error/index.js:8
Warning: Do not await the result of calling ReactTestUtils.act(...), it is not a Promise.
@ValentinH depends on your tests really. in most cases (and after the helpers are wrapped with act), you probably won't, but in the odd case when you do something manually and the warning pops up, you can wrap it by yourself with act().
@KayBeSee rtl hasn't integrated the new async version of act() into its helpers yet. if you can make a git repo wit the above code, I'd be happy to have a look and send a PR which makes your tests pass.
I spent time going through a bunch of issues reported on the react repo and replied with solutions to the same facebook/react#14769 (comment) I want to do the same now for the examples reported in this issue, but feel free to have a look there to see if it helps any of you.
Hey @threepointone, here is a little repo demonstrating the warnings I'm getting b/c of act:
https://github.com/KayBeSee/rtl-act-warning-example
Thank you so much for helping!
Ahh, forgot to update react-dom. Thanks! Working perfectly so far!
@panjiesw - I hope your issue got resolved on the react repo thread
let greetingNode
await act(() => {
const greetingNode = await waitForElement(() => getByTestId('greeting'))
})
expect(fakeAxios.get).toHaveBeenCalledTimes(2) // React runs the render twice in dev mode
expect(fakeAxios.get).toHaveBeenCalledWith(url)
expect(greetingNode).toHaveTextContent('hello there')would pass your tests. or you could integrate it into waitForElement :)
@alexknipfer - from another react repo issue (I forget which one) I assume you're also unlocked, but for posterity's sake
await act(() => {
const receivedElement = await waitForElement(
() => getByText(/^Received:/).textContent
)
expect(receivedElement).toBe(`Received: my test text`)
})await act(() => {
const elements = await waitForElement(() => getAllByTestId(dataTestIdItem));
expect(elements.length).toBe(items.length);
})as you can all see, it's much simpler, and you don't need to futz about with fake timers / build configs, it should all Just Work ™️. I've started writing out my explainer doc (and it's actually exciting me haha), but feel free to reach out if you need something.
@FredyC if you could try out the new alpha with mobx-react-lite and let me know if it fixes your issues, or if you're still having trouble, that would be great. I'd be happy to pitch in and help.
@threepointone Yea, don't worry, the mobx-react-lite is not async, so I only had issues initially mostly with understanding the concept of act, but then it worked fine.
@kentcdodds Feel free to close it here if you want to track integration of async act somewhere else.
I'm going to leave this open.
I plan on implementing support for this today.
Almost there: #343
you're a beast kent
🎉 This issue has been resolved in version 6.1.0 🎉
The release is available on:
npm package (@latest dist-tag)- GitHub release
Your semantic-release bot 📦🚀
Ok so here's the scoop: If you want to get rid of the warnings, upgrade to react-dom@16.9.0-alpha.0 or wait until react-dom@16.9.0 is stably released, and then upgrade. It may be a few weeks before 16.9.0 is actually released, so keep that in mind.
If you don't want to upgrade, but still want to silence the warnings for now, then you can add this to your setupTests.js file (or equivalent):
// this is just a little hack to silence a warning that we'll get until react
// fixes this: https://github.com/facebook/react/pull/14853
const originalError = console.error
beforeAll(() => {
console.error = (...args) => {
if (/Warning.*not wrapped in act/.test(args[0])) {
return
}
originalError.call(console, ...args)
}
})
afterAll(() => {
console.error = originalError
})Then you can remove that when you do upgrade.
I think I'll leave this open until react-dom@16.9.0 is released and we can close this as soon as we remove this: https://github.com/kentcdodds/react-testing-library/blob/180179e12239d7de7ec5bf5f572291cd9ec48d33/src/react-dom-16.9.0-is-released.js 😅
Any time you call a state updater function like
this.setState, ordispatchorsetState, if that's not happening within the call stack of a batch update in react then you'll get this warning
I'm coming a little late, but this is exactly what caused the issue for me. Calling two different setState in sequence (coming from two different useState) got me the act error.
I could get around this problem by wrapping those calls in the unstable_batchedUpdates API from react-dom (which is what act does behind the scenes, I suppose ?).
I think this can be used as a workaround for now (after all, this is not as unstable as its name suggests, and react-redux is relying on it).
I was super confused as to how to solve the issue on my project.
I did manage to find out that it was two useState's setter calls causing this issue.
My code looked something like this:
setSomestuff(true)
change('myFieldName', 'Some value'); // redux form function
await somethingThatCallsAxiosAndDoesAsyncStuff();
setSomestuff(false)☝️All that is inside a useEffect hook though.
At the point of declaration for setSomestuff:
const [ someStuff, setSomeStuff ] = useState(false);It appears that somethingThatCallsAxiosAndDoesAsyncStuff would resolve almost instantly in the tests, which does make sense to me (stuff is mocked, no latency involved), and effectively make it seem like both setStates were being called together.
Doing the following (👇) resolved my problem, without any other changes at all. Didn't even have to use act.
However, I don't completely understand the consequences of doing this. I think it should work just fine, and I sort of understand that the code inside this setTimeout block will be executed in the next first available time slot (or something).
setTimeout(() => setSomestuff(true));
change('myFieldName', 'Some value'); // redux form function
await somethingThatCallsAxiosAndDoesAsyncStuff();
setTimeout(() => setSomestuff(false));But I've spent an hour or two on this reading through solutions that either didn't work, or I didn't know how to implement, and this worked, so I'll take it, for now.
@jayantbh I think this just confirms that this act problem comes from calling useState's setters several times on the same call stack.
With those setTimeouts, this problem disappears because each of their callback will generate their own call stack.
EDIT : would it help if you did something like this instead :
setSomestuff(true)
change('myFieldName', 'Some value'); // redux form function
somethingThatCallsAxiosAndDoesAsyncStuff().then(() => {
setSomestuff(false)
});It appears that somethingThatCallsAxiosAndDoesAsyncStuff would resolve almost instantly in the tests, which does make sense to me (stuff is mocked, no latency involved), and effectively make it seem like both setStates were being called together.
This is a situation I called out in https://github.com/alexkrolick/react-testing-examples
I'll revisit now that async act exists, but I think if you want to simulate the actual timing in the app you need to use fake timers because the intermediate state would otherwise be batched
We were facing this issue in our tests on React 16.8.2.
Similarly to above, the issue is that were using code splitting import('./pin.svg') to dynamically load our svg icons. We were using const [icon, setIcon] = useState() then setting the icon once it was loaded. It worked in the browser but during testing we were getting the above errors our tests.
After some research, we found out that the error was happening because of a babel plugin dynamic-import-node. This basically transforms any dynamic imports into Promise.resolve(require(fileName)) which would resolve immediately.
To get around the problem, we ended up using the console suppression method above. Upgrading to React 16.9 also fixes the issue, but we can't use an alpha release.
Another solution would be to make a babel plugin similar to dynamic-import-node but that resolves after a couple milliseconds (to avoid immediate re-render).
Thanks for the awesome lib!
I read this thread a few times but still havnt found an answer. It seems like we are not supposed to wrap anything in act() but im getting an error
Warning: An update to MyComponentName 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 */
im running latest react-testing-library, latest react-dom (even tried alpha release) but still running into issue with the act warning.
heres my test:
test('some test should render correct content', async () => {
SomeMethodForMockAPI.mockResolvedValue(getApiResponse());
const { container, getByText } = render(<RenderComponent />);
let isLoadingComponent = container.querySelector('[aria-busy="true"]');
expect(isLoadingComponent.length).not.toBeNull();
await waitForElement(() => getByText('Some Text...'), { container });
isLoadingComponent = container.querySelector('[aria-busy="true"]');
expect(isLoadingComponent).toBeNull();
});Could you make a codesandbox that reproduces this issue and post it to the spectrum?
I get the same error when I try to put some async code inside the act callback
Just ran into this with react-testing-library! We were testing a React component that did a state update after a Promise.resolve(). It looked like this:
const MyComponent = () => {
const [ data, setData ] = useState({})
useEffect(() => {
// Fetch data on mount, then save it to the state
fetchData().then(setData)
}, [])
/* ... */
}// Our mock data fetcher was using Promise.resolve()
const fetchData = jest.fn(() => {
return Promise.resolve('mock data goes here')
})This gave us the act warning. This comment tipped me as to how/why this happens:
But if the async call is immediately run in the first render, then it'll generate the warning
The workaround we ended up using is to avoid using Promise.resolve() to make immediately-resolving promises, and instead use a setTimeout() to delay the resolution of these promises. For us, we simply rewrote our fetchData like so:
// Works like Promise.resolve(), but waits a small timeout
const resolve = (value) => {
return new Promise(resolve => setTimeout(resolve)).then(() => value)
}
const fetchData = jest.fn(() => {
return resolve('mock data goes here')
})This also has the unintentional benefit of more realistic tests.
EDIT: Apparently this doesn't always fix it, it still happens. Consider trying jest.useFakeTimers() to arrest setTimeout calls, then using act(() => { jest.runAllTimers() }), but we couldn't get this to work.
I read somewhere in one of the issues that render is automatically wrapped in act. However, I was trying it out and I'm getting the error mentioned by other here
@skippednote Common mistake, you don't have React 16.9.0-alpha.0 which includes handling async stuff. It's kinda sad that React team hasn't made this more clear, I see this mistake all around. Perhaps a small blog post would help there.
If you don't want to upgrade to React 16.9.0-alpha.0 and want to get rid of these messages you can use this snippet:
utils.tsx
import { act as reactAct } from 'react-dom/test-utils';
const SUPPRESSED_PREFIXES = [
"Warning: Do not await the result of calling ReactTestUtils.act(...)",
"Warning: An update to %s inside a test was not wrapped in act(...)",
];
function isSuppressedErrorMessage(message: string): boolean {
return SUPPRESSED_PREFIXES.some(sp => message.startsWith(sp));
}
export async function act(f: () => void): Promise<void> {
const oldError = window.console.error;
window.console.error = (...args: any[]) => {
if (!isSuppressedErrorMessage(args[0])) {
oldError(...args);
}
};
await Promise.race([reactAct(f), new Promise(res => setTimeout(res))]);
window.console.error = oldError;
}YourComponent.test.tsx
import React from 'react';
import ReactDOM from 'react-dom';
import { act } from './utils';
import MyComponent from './MyComponent';
it('renders without crashing', async () => {
const div = document.createElement('div');
await act(() => {
ReactDOM.render(<MyComponent />, div);
});
});Once React with working act is available you can just change your utils.tsx to:
export { act } from 'react-dom/test-utils';as, it will be OK to await on act in the new version. Tou will seamlessly switch to using the React version without much changes in your code.
Examples are in TypeScript, but porting these to JavaScript should not be a problem :)
[...] you can add this to your setupTests.js file (or equivalent):
const originalError = console.error beforeAll(() => { ... }) afterAll(() => { ... })
Found a problem with this approach: if you mock console.error in a test and call expect(console.error).toHaveBeenCalledTimes(1) your test won't work anymore. This is what I do instead:
// jest.config.js
module.exports = {
// or setupFilesAfterEnv
setupFiles: ['./jest.setup.js'],
...
};// jest.setup.js
// FIXME Remove when we upgrade to React >= 16.9
const originalConsoleError = console.error;
console.error = (...args) => {
if (/Warning.*not wrapped in act/.test(args[0])) {
return;
}
originalConsoleError(...args);
};