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

Add relevant stacktrace to act warnings #18359

Closed
kentcdodds opened this issue Mar 20, 2020 · 7 comments
Closed

Add relevant stacktrace to act warnings #18359

kentcdodds opened this issue Mar 20, 2020 · 7 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@kentcdodds
Copy link

React version: ^16.8.0

Steps To Reproduce

  1. set state without wrapping in act
  2. observe the act warning

Link to code example:

The current behavior

Right now the act warning gives a component stack trace which is nice, but in the age of reusable hooks, it's not enough to help people really identify where the issue is because it could be in hooks far away from the component.

Here's an example of the output:

  console.error node_modules/react-dom/cjs/react-dom.development.js:530
    Warning: An update to Comp 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 Comp (at example.test.js:23)

The expected behavior

I think it would be nice if a stack trace were included in addition to the component stack trace. I would expect that the stack would only include non-react code.

Here's an example of what I think would look good:

  console.error node_modules/react-dom/cjs/react-dom.development.js:530
    Warning: An update to Comp 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 Comp (at example.test.js:23)
    
    Stack Trace: 
        at Object.<anonymous> (/Users/kentcdodds/Desktop/0mv31/src/example.test.js:25:3)

In a more real world scenario (like one I'm dealing with right now) here's what it might look like:

  console.error node_modules/react-dom/cjs/react-dom.development.js:88
    Warning: An update to StatusButtons 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 StatusButtons (at book.js:69)
        in div (created by Context.Consumer)
        in EmotionCssPropInternal (at book.js:59)
        in div (created by Context.Consumer)
        in EmotionCssPropInternal (at book.js:50)
        in div (at book.js:49)
        in div (created by Context.Consumer)
        in EmotionCssPropInternal (at book.js:32)
        in div (at book.js:31)
        in BookScreen (at book.js:33)
        in MockAuthProvider (at context/index.js:15)
        in Router (created by BrowserRouter)
        in BrowserRouter (at context/index.js:14)
        in ReactQueryConfigProvider (at context/index.js:13)
        in AppProviders (at app-test-utils.js:9)
        in Wrapper
    
    Stack Trace: 
        at rerender (/Users/kentcdodds/code/bookshelf/node_modules/react-query/src/utils.js:119:45)
        at Object.onStateUpdate (/Users/kentcdodds/code/bookshelf/node_modules/react-query/src/useBaseQuery.js:67:28)
        at forEach (/Users/kentcdodds/code/bookshelf/node_modules/react-query/src/queryCache.js:224:38)
        at Array.forEach (<anonymous>)
        at dispatch (/Users/kentcdodds/code/bookshelf/node_modules/react-query/src/queryCache.js:224:23)
        at /Users/kentcdodds/code/bookshelf/node_modules/react-query/src/queryCache.js:234:11

That would be an enormous help because the offending code is not my own but one of my dependencies.

Thanks!

@kentcdodds kentcdodds added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Mar 20, 2020
@kentcdodds
Copy link
Author

Oh, and here's a reproduction: https://github.com/kentcdodds/act-stack-example

@gaearon
Copy link
Collaborator

gaearon commented Mar 20, 2020

Isn’t this something Jest should do? The browser does it.

@gaearon
Copy link
Collaborator

gaearon commented Mar 20, 2020

Also the repo is empty. :-)

@kentcdodds
Copy link
Author

Oh, sorry about that. It was basically this: https://codesandbox.io/s/angry-curran-0mv31

Except codesandbox doesn't always work well with tests so feel free to download that to investigate further.

And if the browser does this then yes, this is probably something Jest should do. Any chance you could transfer this issue?

@bvaughn
Copy link
Contributor

bvaughn commented Mar 20, 2020

To transfer the issue, we'd need permissions in the Jest repo too 😆 and I, at least, don't have them.

@gaearon
Copy link
Collaborator

gaearon commented Mar 20, 2020

I have a strong deja vu

jestjs/jest#8819

@gaearon gaearon closed this as completed Mar 20, 2020
@kentcdodds
Copy link
Author

lol 😂 I totally forgot about that

Sorry for the trouble 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

3 participants