-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Mounted wrapper was not wrapped in act(...) warning #2073
Comments
I just stumbled on this issue while looking into this StackOverflow question. The warning seems to be triggered by asynchronous calls to Here is an extremely simple import React, { useState, useEffect } from 'react';
import { mount } from 'enzyme';
const SimpleComponent = () => {
const [data, setData] = useState('initial');
useEffect(() => {
setImmediate(() => setData('updated'));
}, []);
return (<div>{data}</div>);
};
test('SimpleComponent', done => {
const wrapper = mount(<SimpleComponent/>);
setImmediate(done);
}); ...which results in this warning:
|
I'm having the same problem :( |
@brian-lives-outdoors That specific issue can be resolved by wrapping |
Do you have a git repo where I can try this out? FYI, we just released an alpha (https://github.com/facebook/react/releases/tag/v16.9.0-alpha.0) with an async version of |
I'm getting this for the very first time as well. I'm just testing I'm guessing this will be resolved when React hooks are supported in Enzyme. My tests still work. |
@threepointone fwiw, it's actually a problem that Separately, the next release of enzyme should address this issue (#2073). |
I wouldn’t expect act to work with shallow renderer anyway (or for it to even have the warning, because shallow renderer doesn’t do updates I think?) can any of you share a repo with a failing test that I could have a look at? |
@threepointone that's part of a longer conversation, i think - the shallow renderer doesn't do updates, but enzyme's shallow does. |
The thing is, act() is renderer specific (there’s one each for test-utils, test-renderer, and the noop-renderers.) there isn’t one for the shallow-renderer because we don’t do updates or effects for it. I’m curious to see how you’d get the warning, hence asking for a failing test repro. Maybe it would still ‘work’ because of its internals, but I can’t be sure without seeing. Lemme know if you’d like me to have a look. |
we're using the one from test-utils atm, for mount. I was trying to use the same one for shallow, not realizing it was renderer-specific. It didn't work because it required a DOM, and shallow must work in a node environment. |
@threepointone I'm afraid the code is in a private repo belonging to my work so I can't share it. I'm currently on holiday, however, I'll try and replicate the issue in a codepen or something next week. |
This is addressed by #2034, so I'm going to close it - it will be in the next release. |
when will be the next release? My team is waiting for this fix for one month already... |
v3.10.0 has now been released. |
@vinyfc93 that’s because that PR only affected the react 16 adapter, and has been published for months. Ensure that’s updated, and if you’re still having trouble, please file a new issue. |
@ljharb I have ensured both
Since this seems to be the same issue as the one above should a new issue be opened? |
Yes, please. |
The same problem after update console.error node_modules/react-dom/cjs/react-dom.development.js:506
|
@albertly you still have to manually wrap some kinds of changes with |
## This change includes: - Refactoring Tooltip to be a function component, using hooks for state and effects - Remove the intermediary EscapableTooltip (Fixes #354) - Update all the unit tests to avoid using `setState()` and `state()` in favour of simulating clicks and asserting the DOM directly. This is compatible with hooks, and is a better design of black-box testing - Split out server tests and run them in Jest's server environment - Upgrade Enzyme (only a minor version bump) to support new Hooks testing features (see enzymejs/enzyme#2073)
I also have this problem, using: "enzyme": "^3.10.0",
"enzyme-adapter-react-16": "^1.14.0" This seems still to be an issue? Could this be opened again or am I missing something? |
@richarddd please file a new issue. |
I suspect for many the issue is still present due to a limitation with const mockConsoleMethod = (realConsoleMethod) => {
const ignoredMessages = [
'test was not wrapped in act(...)',
]
return (message, ...args) => {
const containsIgnoredMessage = ignoredMessages.some(ignoredMessage => message.includes(ignoredMessage))
if (!containsIgnoredMessage) {
realConsoleMethod(message, ...args)
}
}
}
console.warn = jest.fn(mockConsoleMethod(console.warn))
console.error = jest.fn(mockConsoleMethod(console.error)) It's not ideal but whatever I want to be able to test my hook components. 😉 |
I also have this problem, with : |
Using TS-- an even simpler version of @SenP 's solution if you don't need to interact with the page and don't want to install waait is this: const waitForComponentToPaint = async (wrapper: any) => {
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 0));
wrapper.update();
});
}; Usage: it('should do something', () => {
const wrapper = mount(<MyComponent ... />);
waitForComponentToPaint(wrapper);
expect(wrapper).toBlah...
}) This is obviously not a good general solution-- looking forward to the general fix |
Thanks @edpark11 – your solution is the cleanest for me until something official upstream. Added some slight type safety: import { ReactWrapper } from 'enzyme';
import { act } from 'react-dom/test-utils';
export async function waitForComponentToPaint<P = {}>(
wrapper: ReactWrapper<P>,
amount = 0,
) {
await act(async () => {
await new Promise(resolve => setTimeout(resolve, amount));
wrapper.update();
});
} |
In my case, it seems it('render a LocationSelector', () => {
jest.useFakeTimers()
const spy = jest.spyOn(console, 'error')
spy.mockImplementation(() => {})
const store = mockStore({
common: { isInternetReachable: true }
})
const getLocation = () => jest.fn()
const close = () => jest.fn()
const isVisible = true
const wrapper = mount(
<Provider store={store}>
<LocationSelector
isVisible={isVisible}
getLocation={getLocation}
close={close}
/>
</Provider>
)
act(() => {
jest.runAllImmediates()
})
spy.mockRestore()
}) |
enzymejs/enzyme#2073 seems to be an ongoing issue, and causes components with useEffect to update after the test is completed. waitForUpdates ensures that updates have completed within an act() before continuing on.
* Add Frontend components for Value Lists Management Modal Imports and uses the hooks provided by the lists plugin. Tests coming next. * Update value list components to use newest Lists API * uses useEffect on a task's state instead of promise chaining * handles the fact that API calls can be rejected with strings * uses exportList function instead of hook * Close modal on outside click * Add hook for using a cursor with paged API calls. For e.g. findLists, we can send along a cursor to optimize our query. On the backend, this cursor is used as part of a search_after query. * Better implementation of useCursor * Does not require args for setCursor as they're already passed to the hook * Finds nearest cursor for the same page size Eventually this logic will also include sortField as part of the hash/lookup, but we do not currently use that on the frontend. * Fixes useCursor hook functionality We were previously storing the cursor on the _current_ page, when it's only truly valid for the _next_ page (and beyond). This was causing a few issues, but now that it's fixed everything works great. * Add cursor to lists query This allows us to search_after a previous page's search, if available. * Do not validate response of export This is just a blob, so we have nothing to validate. * Fix double callback post-import After uploading a list, the modal was being shown twice. Declaring the constituent state dependencies separately fixed the issue. * Update ValueListsForm to manually abort import request These hooks no longer care about/expose an abort function. In this one case where we need that functionality, we can do it ourselves relatively simply. * Default modal table to five rows * Update translation keys following plugin rename * Try to fit table contents on a single row Dates were wrapping (and raw), and so were wrapped in a FormattedDate component. However, since this component didn't wrap, we needed to shrink/truncate the uploaded_by field as well as allow the fileName to truncate. * Add helper function to prevent tests from logging errors enzymejs/enzyme#2073 seems to be an ongoing issue, and causes components with useEffect to update after the test is completed. waitForUpdates ensures that updates have completed within an act() before continuing on. * Add jest tests for our form, table, and modal components * Fix translation conflict * Add more waitForUpdates to new overview page tests Each of these logs a console.error without them. * Fix bad merge resolution That resulted in duplicate exports. * Make cursor an optional parameter to findLists This param is an optimization and not required for basic functionality. * Tweaking Table column sizes Makes actions column smaller, leaving more room for everything else. * Fix bug where onSuccess is called upon pagination change Because fetchLists changes when pagination does, and handleUploadSuccess changes with fetchLists, our useEffect in Form was being fired on every pagination change due to its onSuccess changing. The solution in this instance is to remove fetchLists from handleUploadSuccess's dependencies, as we merely want to invoke fetchLists from it, not change our reference. * Fix failing test It looks like this broke because EuiTable's pagination changed from a button to an anchor tag. * Hide page size options on ValueLists modal table These have style issues, and anything above 5 rows causes the modal to scroll, so we're going to disable it for now. * Update error callbacks now that we have Errors We don't display the nice errors in the case of an ApiError right now, but this is better than it was. * Synchronize delete with the subsequent fetch Our start() no longer resolves in a meaningful way, so we instead need to perform the refetch in an effect watching the result of our delete. * Cast our unknown error to an Error useAsync generally does not know how what its tasks are going to be rejected with, hence the unknown. For these API calls we know that it will be an Error, but I don't currently have a way to type that generally. For now, we'll cast it where we use it. * Import lists code from our new, standardized modules Co-authored-by: Elastic Machine <[email protected]>
* Add Frontend components for Value Lists Management Modal Imports and uses the hooks provided by the lists plugin. Tests coming next. * Update value list components to use newest Lists API * uses useEffect on a task's state instead of promise chaining * handles the fact that API calls can be rejected with strings * uses exportList function instead of hook * Close modal on outside click * Add hook for using a cursor with paged API calls. For e.g. findLists, we can send along a cursor to optimize our query. On the backend, this cursor is used as part of a search_after query. * Better implementation of useCursor * Does not require args for setCursor as they're already passed to the hook * Finds nearest cursor for the same page size Eventually this logic will also include sortField as part of the hash/lookup, but we do not currently use that on the frontend. * Fixes useCursor hook functionality We were previously storing the cursor on the _current_ page, when it's only truly valid for the _next_ page (and beyond). This was causing a few issues, but now that it's fixed everything works great. * Add cursor to lists query This allows us to search_after a previous page's search, if available. * Do not validate response of export This is just a blob, so we have nothing to validate. * Fix double callback post-import After uploading a list, the modal was being shown twice. Declaring the constituent state dependencies separately fixed the issue. * Update ValueListsForm to manually abort import request These hooks no longer care about/expose an abort function. In this one case where we need that functionality, we can do it ourselves relatively simply. * Default modal table to five rows * Update translation keys following plugin rename * Try to fit table contents on a single row Dates were wrapping (and raw), and so were wrapped in a FormattedDate component. However, since this component didn't wrap, we needed to shrink/truncate the uploaded_by field as well as allow the fileName to truncate. * Add helper function to prevent tests from logging errors enzymejs/enzyme#2073 seems to be an ongoing issue, and causes components with useEffect to update after the test is completed. waitForUpdates ensures that updates have completed within an act() before continuing on. * Add jest tests for our form, table, and modal components * Fix translation conflict * Add more waitForUpdates to new overview page tests Each of these logs a console.error without them. * Fix bad merge resolution That resulted in duplicate exports. * Make cursor an optional parameter to findLists This param is an optimization and not required for basic functionality. * Tweaking Table column sizes Makes actions column smaller, leaving more room for everything else. * Fix bug where onSuccess is called upon pagination change Because fetchLists changes when pagination does, and handleUploadSuccess changes with fetchLists, our useEffect in Form was being fired on every pagination change due to its onSuccess changing. The solution in this instance is to remove fetchLists from handleUploadSuccess's dependencies, as we merely want to invoke fetchLists from it, not change our reference. * Fix failing test It looks like this broke because EuiTable's pagination changed from a button to an anchor tag. * Hide page size options on ValueLists modal table These have style issues, and anything above 5 rows causes the modal to scroll, so we're going to disable it for now. * Update error callbacks now that we have Errors We don't display the nice errors in the case of an ApiError right now, but this is better than it was. * Synchronize delete with the subsequent fetch Our start() no longer resolves in a meaningful way, so we instead need to perform the refetch in an effect watching the result of our delete. * Cast our unknown error to an Error useAsync generally does not know how what its tasks are going to be rejected with, hence the unknown. For these API calls we know that it will be an Error, but I don't currently have a way to type that generally. For now, we'll cast it where we use it. * Import lists code from our new, standardized modules Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
A previous commit introduced waitForUpdates as a solution to the warnings introduced by enzymejs/enzyme#2073: by waiting for the effects to complete we avoid the warning. However, waiting for the effects to complete could occasionally be very costly, especially on an overtasked CI machine, and I've been seeing these tests fail on occasion due to timeouts. Since a warning message is preferable to a false negative, I'm removing waitForUpdates and allowing the warnings to occur, as this should be fixed on a subsequent update of enzyme/react-adapter. I've also fixed warnings in a few particularly problematic/noisy tests by simply unmounting the component at the end of the test (this does not work in an afterEach).
* Revert "Skip jest tests that timeout waiting for react" This reverts commit dd9b0b3. * Unmount async effectful components instead of waiting for them A previous commit introduced waitForUpdates as a solution to the warnings introduced by enzymejs/enzyme#2073: by waiting for the effects to complete we avoid the warning. However, waiting for the effects to complete could occasionally be very costly, especially on an overtasked CI machine, and I've been seeing these tests fail on occasion due to timeouts. Since a warning message is preferable to a false negative, I'm removing waitForUpdates and allowing the warnings to occur, as this should be fixed on a subsequent update of enzyme/react-adapter. I've also fixed warnings in a few particularly problematic/noisy tests by simply unmounting the component at the end of the test (this does not work in an afterEach).
* Revert "Skip jest tests that timeout waiting for react" This reverts commit dd9b0b3. * Unmount async effectful components instead of waiting for them A previous commit introduced waitForUpdates as a solution to the warnings introduced by enzymejs/enzyme#2073: by waiting for the effects to complete we avoid the warning. However, waiting for the effects to complete could occasionally be very costly, especially on an overtasked CI machine, and I've been seeing these tests fail on occasion due to timeouts. Since a warning message is preferable to a false negative, I'm removing waitForUpdates and allowing the warnings to occur, as this should be fixed on a subsequent update of enzyme/react-adapter. I've also fixed warnings in a few particularly problematic/noisy tests by simply unmounting the component at the end of the test (this does not work in an afterEach).
* Revert "Skip jest tests that timeout waiting for react" This reverts commit dd9b0b3. * Unmount async effectful components instead of waiting for them A previous commit introduced waitForUpdates as a solution to the warnings introduced by enzymejs/enzyme#2073: by waiting for the effects to complete we avoid the warning. However, waiting for the effects to complete could occasionally be very costly, especially on an overtasked CI machine, and I've been seeing these tests fail on occasion due to timeouts. Since a warning message is preferable to a false negative, I'm removing waitForUpdates and allowing the warnings to occur, as this should be fixed on a subsequent update of enzyme/react-adapter. I've also fixed warnings in a few particularly problematic/noisy tests by simply unmounting the component at the end of the test (this does not work in an afterEach). Co-authored-by: Elastic Machine <[email protected]>
* Revert "Skip jest tests that timeout waiting for react" This reverts commit dd9b0b3. * Unmount async effectful components instead of waiting for them A previous commit introduced waitForUpdates as a solution to the warnings introduced by enzymejs/enzyme#2073: by waiting for the effects to complete we avoid the warning. However, waiting for the effects to complete could occasionally be very costly, especially on an overtasked CI machine, and I've been seeing these tests fail on occasion due to timeouts. Since a warning message is preferable to a false negative, I'm removing waitForUpdates and allowing the warnings to occur, as this should be fixed on a subsequent update of enzyme/react-adapter. I've also fixed warnings in a few particularly problematic/noisy tests by simply unmounting the component at the end of the test (this does not work in an afterEach).
use your methods,I still get the error console.error
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. |
timeouts and retires are the worse way to test anything. I can't stand tests with timeouts in them. Cypress is notorious for this. I found that this is really the only way you can test Hooks or use spies, both of which are horrendous ways to write tests. Flaky tests, slow tests, neither is good and is a result of writing any kind of test this way. It's unfortunate. The React Testing Lib has waitFor which is just sugar to say "Lets retry based in timeouts" which is a terrible way to test components. The React Testing Library overall is really "Let me provide you some more sugar over the bad, to make you feel like it's better testing but it really isn't". Without a component lifecycle like Class components had that allowed us to observe and find out truly when an internal async call was resolved the first time (which I believe is what enzyme was able to do with class components), we're stuck with really bad routes to test Hooks no matter what testing lib we're using. Also, If I'm not wrong, you have to test on second render of a Hook, because useEffect which if you call setState, updates state to cause a re-render, is not called till after you mount or render it the first time. So the lack of a lifecycle to observe and the fact that we're stuck with how React forces Hooks to run (first render, then re-render with useEffect), we're stuck with either these two routes to test:
|
This is my solution without setTimout Helper: import {waitFor} from "@testing-library/react"
import {ReactElement} from "react";
import {ReactWrapper} from "enzyme";
interface asyncRenderModel {
(renderMethod: (node: ReactElement) =>
ReactWrapper<any>, component: ReactElement): Promise<ReactWrapper<any>>
}
const asyncRender: asyncRenderModel = async (renderMethod, component) => {
let result = renderMethod(<></>);
try {
await waitFor(() => {
result = renderMethod(component)
})
} catch (e) {
console.error(e)
}
return result;
}
export default asyncRender How to use: test('Should render component', async () => {
const app = await asyncRender(mount, <Component/>);
expect(app.find(SomeElement)).toHaveLength(1)
}) |
That's because the await was missing inside the test ala |
just use isolate-components to test hooks. Way better than RTL, less verbose, etc.. I'm using isolate-components for WeDoTDD.com and it's working just great. I'm done with enzyme at least for testing hooks. I still use enzyme for Class React components (not a fan of RTL period) but that's about it these days. Also people, get rid of |
I provided a const React = require('react');
module.exports = {
...React,
useState: (...args) => {
const { act } = require('react-dom/test-utils');
const [state, setState] = React.useState(...args);
const actSetState = (...newStateArgs) => {
act(() => setState(...newStateArgs));
};
return [state, actSetState];
}
}; I've placed this wherever the jest mocks are located (I'm pretty unfamiliar with jest, sorry). So essentially for your tests jest would resolve react to this object, which has a version of |
This is still a problem. Using React 16.14.0 with up-to-date versions of Enzyme and JEST.
This does clear the warnings, but how can I test for my application's appearance/state before the state changes in the |
Using @edpark11's solution works! Just a note, the
|
## This change includes: - Refactoring Tooltip to be a function component, using hooks for state and effects - Remove the intermediary EscapableTooltip (Fixes #354) - Update all the unit tests to avoid using `setState()` and `state()` in favour of simulating clicks and asserting the DOM directly. This is compatible with hooks, and is a better design of black-box testing - Split out server tests and run them in Jest's server environment - Upgrade Enzyme (only a minor version bump) to support new Hooks testing features (see enzymejs/enzyme#2073)
This comment was marked as spam.
This comment was marked as spam.
Put this after the await act(() => {
wrapper.update();
}); |
Current behaviour
I have a component that makes use of the
useEffect
hook to make an asynchronous call an api (or in the test case a mock of a call to an api) and then call thesetState
hook with the result.I've written tests that assert on the outcome of the effect (ie content being returned from the api). I'm using v1.11.2 of the adapter for react-16 my tests pass, snapshots are created and everything appears to be working well, however, a warning is repeatedly logged out from
react-dom
.Here's the test in question:
Here's the warning:
I've attempted wrapping the call to
enzyme.mount(<LinkBlockComponent id={1} getLinkBlock={apiMock} />);
in an act block, and asserting afterwards as the warning suggests but this ends up calling all my tests before the component has finished rendering.I don't suspect this is an issue caused enzyme or the adapter itself as I've noticed that you are wrapping mount calls within an act() block further down, however, I've struggled to find any documentation from enzyme on testing the effect of
useEffect
.If you have any suggestions on best practices here or if anything looks amiss, help would be appreciated.
Expected behaviour
Your environment
API
Version
Adapter
The text was updated successfully, but these errors were encountered: