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

Hooks stable #10

Merged
merged 8 commits into from Apr 4, 2019
Merged

Conversation

NateBrady23
Copy link
Member

Before getting this in there's an issue that needs to be addressed. We now receive the following warning for all the tests:

      Warning: An update to Counter 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

https://fb.me/react-wrap-tests-with-act

The tests all still pass, but I attempted to use act from react-test-renderer and wrapping everything in it and the tests failed because act won't wait on async events. Trying something like:

act(async () => {
  await button.props.onClick();
});

Works but we get another warning that act shouldn't return anything. (In this case, a promise).

The examples in the documentation linked above are pretty standard click events that don't rely on async updates to state. We need to figure out the right way to do this before moving forward.

@NateBrady23
Copy link
Member Author

NateBrady23 commented Feb 7, 2019

Tracking facebook/react#14775

Edit: They're aware of the problem and working on a solution. Let's see if they get one out fast and I'll update the test cases before merging.

package.json Outdated
"react-test-renderer": "16.8.0-alpha.1",
"react": "16.8.1",
"react-dom": "16.8.1",
"react-test-renderer": "16.8.1",
Copy link
Member

Choose a reason for hiding this comment

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

I am curious why would we use ^16.8.0 in peer dependencies but lock it to 16.8.1 for dev deps?

Copy link
Member Author

Choose a reason for hiding this comment

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

Peer dependencies is the minimum required version of React for this library to work. However, this can still be ^16.8.1 for dev dependencies. Didn't mean to lock it down but wanted to bump to latest for warnings while doing library dev.

@NateBrady23
Copy link
Member Author

facebook/react#14853

@msmith-techempower msmith-techempower merged commit e61dd39 into TechEmpower:master Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants