Skip to content

regression: console.error supression no longer works in 0.5 #50

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

Closed
ntucker opened this issue Apr 24, 2019 · 6 comments
Closed

regression: console.error supression no longer works in 0.5 #50

ntucker opened this issue Apr 24, 2019 · 6 comments
Labels
bug Something isn't working

Comments

@ntucker
Copy link
Contributor

ntucker commented Apr 24, 2019

  • react-hooks-testing-library version: 0.5.0
  • react-testing-library version: 6.1.2
  • react version: 16.8.6
  • node version: 11.10
  • npm (or yarn) version: 1.15.2

Relevant code or config:

facebook/react#11098 (comment)

let your = (code, tell) => `the ${story}`;

What you did:

function onError(e: any) {
  e.preventDefault();
}
beforeEach(() => {
  window.addEventListener('error', onError);
});
afterEach(() => {
  window.removeEventListener('error', onError);
});

What happened:

console.error node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9215
The above error occurred in the component:
in TestHook
in Suspense
in ErrorBoundary
in Suspense (created by wrapper)
in ExternalCacheProvider
in Unknown (created by wrapper)
in wrapper

React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary.

Reproduction:

Problem description:

This used to suppress the console errors from things thrown in render being caught by errorboundaries. It doesn't work anymore

Suggested solution:

@ntucker ntucker added the bug Something isn't working label Apr 24, 2019
@mpeyper
Copy link
Member

mpeyper commented Apr 25, 2019

I'll take a look, but the big difference between the versions was changing from react-testing-library (which uses react-dom under the hood) to using react-test-renderer to handle rendering, and I'm not sure if the fix mentioned in Dan's comment applies to both renderers, or just react-dom.

@ntucker
Copy link
Contributor Author

ntucker commented Apr 25, 2019

Hmm, I wonder what the expected workaround is for react-test-renderer....one would think it shouldn't even trigger this since it's definitely in testing mode.

@mpeyper
Copy link
Member

mpeyper commented Apr 25, 2019

I had a look at the source code for react-test-renderer and the fix is there, but I think it might have a bug.

Putting console logs into their error handler where it checks for a prevented default (which is added as an event listener to window, same as yours) never logs, but a log where it checks for the suppression flag does log.

This tells be that the way errors are handled in the test renderer doesn't trigger window events, which sort of makes sense as the point is it doesn't require a DOM to render.

I'm still investigating, but I see 3 options ahead:

  1. We do nothing and raise a bug with react (assuming it's their bug)
  2. We use react-dom (probably with react-testing-library again) for rendering
  3. We don't use an error boundary for error handling and catch the errors in our test component

@mpeyper
Copy link
Member

mpeyper commented Apr 27, 2019

@ntucker, I have raised the above issue with the React team as I cannot find any way to make it work. Can you confirm whether or not this was working for you in ^0.4.0 as I cannot seem to suppress the error using react-dom either.

If the response from the React team comes back as "you can't" then I think I'll go for option 3 from my last reply. I've also been wondering if it's possible to have renderHook and rerender throw the error and waitForNextUpdate reject the promise in a similar mechanism to how the result.current getter does so that we can do away with the result.error value, which will require a bit of a rewrite on the error handling anyway.

@alexkrolick
Copy link

Hmm, it seems that code that is actually erroring out in the test is not printing the full stack trace due to some combination of the error boundary and error handling in the lib. This makes it difficult to debug. I think I'd go with #3 somehow, to allow errors to propagate.

@mpeyper
Copy link
Member

mpeyper commented Jun 2, 2019

I think I'd go with #3 somehow

Haha @alexkrolick, I just spent way t0o long thinking about how issue #3 related to this situation 😆

@ntucker I've removed the ErrorBoundary component in v0.5.1 so I'll close this now, but feel free to reopen if there are still issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants