Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Wrap in act() to avoid extraneous console.error #134

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

Merged
kentcdodds merged 2 commits into epicweb-dev:main from IanVS:wrap-in-act
Apr 21, 2021

Conversation

@IanVS
Copy link
Contributor

@IanVS IanVS commented Apr 21, 2021
edited
Loading

I was working through exercise 2-extra 3, and was having a hard time getting the tests to pass, even though my implementation seemed to work fine in the browser, without throwing an error.

I added a useState in useAsync to track whether or not the fetch should be considered cancelled, and added a useEffect in the component with a cleanup method to call the setter for that state. The test failed on the last step, and when I replaced the alfredTip with a plain expect(console.error).not.toHaveBeenCalled() I could see this console error:

expect(jest.fn()).not.toHaveBeenCalled()
Expected number of calls: 0
Received number of calls: 1
1: "Warning: An update to %s 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 */·
...

Wrapping an act() around the click and the unmount removes the warning and lets the test pass. This PR also wraps the type, just because that feels cleaner for some reason.

I know my solution wasn't the same as what KCD shows in the video (and it's not as good), but he did leave the instructions vague on purpose, so it seems like the tests should accept anything that works. 😉

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for letting me know of the issue! I made a few changes.

Copy link
Member

With these changes now, the output will include the error output and we avoid the distracting act warning (even though when the implementation is correct act shouldn't be needed). So here's what the output looks like before the implementation is working:

 FAIL src/__tests__/02.extra-3.js
 ✕ displays the pokemon (597 ms)
 くろまる displays the pokemon
 🚨 Make sure that when the component is unmounted the component does not attempt to trigger a rerender with `dispatch`
 console.error
 Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
 at PokemonInfo (/Users/kentcdodds/code/epic-react/advanced-react-hooks/src/final/02.extra-3.js:87:23)
 at ErrorBoundary (/Users/kentcdodds/code/epic-react/advanced-react-hooks/node_modules/react-error-boundary/dist/react-error-boundary.cjs.js:40:35)
 at PokemonErrorBoundary
 at div
 at div
 at App (/Users/kentcdodds/code/epic-react/advanced-react-hooks/src/final/02.extra-3.js:113:47)
 at div
 at AppWithUnmountCheckbox (/Users/kentcdodds/code/epic-react/advanced-react-hooks/src/final/02.extra-3.js:137:41)
 at console.<anonymous> (node_modules/jest-mock/build/index.js:845:25)
 at printWarning (node_modules/react-dom/cjs/react-dom.development.js:67:30)
 at error (node_modules/react-dom/cjs/react-dom.development.js:43:5)
 at warnAboutUpdateOnUnmountedFiberInDEV (node_modules/react-dom/cjs/react-dom.development.js:23914:9)
Test Suites: 1 failed, 1 total
Tests: 1 failed, 1 total
Snapshots: 0 total
Time: 2.34 s
Ran all test suites related to changed files.
Watch Usage: Press w to show more.

Should be much more helpful. Thanks again 👍

@kentcdodds kentcdodds merged commit d5107db into epicweb-dev:main Apr 21, 2021
Copy link
Member

@all-contributors please add @IanVS for tests

Copy link
Contributor

@kentcdodds

I've put up a pull request to add @IanVS! 🎉

@IanVS IanVS deleted the wrap-in-act branch April 21, 2021 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kentcdodds kentcdodds kentcdodds approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /