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

Difficult to track down the source of a boolean prop warning #11083

Closed
erikras opened this issue Oct 4, 2017 · 11 comments
Closed

Difficult to track down the source of a boolean prop warning #11083

erikras opened this issue Oct 4, 2017 · 11 comments

Comments

@erikras
Copy link

erikras commented Oct 4, 2017

Do you want to request a feature or report a bug?

More of a DX issue, I think. Certainly a frustration in my upgrade. I would not have submitted, but @gaearon told me thrice to do so.

What is the current behavior?

My build works, but is strewn with warnings that I would like to fix. Warnings like:

Warning: Received `false` for non-boolean attribute `dirty`. If this is expected, cast the value to a string.
  in div
  in Unknown (created by Form(Component))
  in Form(Component) (created by Connect(Form(Component)))
  in Connect(Form(Component)) (created by ReduxForm)
  in ReduxForm
  in Provider

My library provides a HOC that provides boolean props. At first, I was concerned that React 16 would no longer allow this, but I think the problem is that one of my tests is carelessly leaking my props all the way into a <div/>. I even created the dumbest HOC ever to test if a HOC could pass a boolean prop, and it seems like it can.

However, from that "stacktrace" (component-trace?), it is not even remotely clear which of my 1630 tests is causing the warning. In previous versions, it would show me the exact stacktrace, with file names and line numbers, that generated the error.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 16.0.0, yes worked with previous.


Additionally, I have several tests that need to throw errors to alert library users that they are doing something wrong, but my test logs are now full of bright red:

Consider adding an error boundary to your tree to customize error handling behavior.
You can learn more about error boundaries at https://fb.me/react-error-boundaries.

Is that just a new fact of life for library developers, or can I catch those in my test somehow?


All this can be replicated with the current master branch of redux-form.

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

However, from that "stacktrace" (component-trace?), it is not even remotely clear which of my 1630 tests is causing the warning. In previous versions, it would show me the exact stacktrace, with file names and line numbers, that generated the error.

Are you sure you’re not confusing JS traces and component traces?

Can you show me a screenshot of what you consider “good behavior”? I’d like to understand what you mean better because I don’t believe this particular warning existed before 16, so I don’t see how it could have a better stack trace before. Or perhaps you were referring to some different warnings that had better stack traces? Can you show a screenshot of the ones that were useful?

Component stack traces (note: these are not JS traces, but component traces) can only show line numbers if you use this plugin in development. Did you enable it? If you did, you’d see where the root element is defined (with a line number), and probably find the test.

My impression from your issue is that you are referring to JS traces. Well, there’s nothing we can do to make them disappear. If you saw them in your browser before, you should see them now as well. Did you click on an “expand” arrow to see them?

Additionally, I have several tests that need to throw errors to alert library users that they are doing something wrong, but my test logs are now full of bright red:

Are you running them in the browser? We haven’t really considered this use case because we always run tests in jsdom. You can comment about it in #10474 (comment).

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

To sum up, we’ll need screenshots “before” and “after” to draw any conclusions :-)

@erikras
Copy link
Author

erikras commented Oct 4, 2017

Yes, obviously these are all new warnings. So your request of before and after is somewhat unfulfillable. But previously when I had errors in my unit tests, it was pretty easy to know exactly where, and now it is not.

We haven’t really considered this use case because we always run tests in jsdom.

My tests are also running in jsdom.

Thanks for the plugin. Trying it out now.

@erikras
Copy link
Author

erikras commented Oct 4, 2017

It's only been a few seconds, but I think that plugin was 💯 what I needed. When I confirm, I'll close this issue.

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

But previously when I had errors in my unit tests, it was pretty easy to know exactly where, and now it is not.

This is not an error, it’s a warning. We never printed JS stacks for warnings. (Although technically it could be possible if you overrode console.error and printed new Error().stack there.)

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

My tests are also running in jsdom.

In this case seeing an additional "error boundary" log doesn't look expected to me. Can you figure out a minimal reproducing project for this?

@erikras
Copy link
Author

erikras commented Oct 4, 2017

I think the problem is that one of my tests is carelessly leaking my props all the way into a <div/>

My suspicion was:

  • 100% correct
  • Nearly impossible to isolate without the plugin ☹️
  • Trivial to find and remedy with the plugin 😁

So I guess I'd recommend advertising that a bit more if not incorporating its functionality into the library itself.

Thanks, Dan!

@erikras erikras closed this as completed Oct 4, 2017
@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

Still, could you share a minimal project that spits out the unhandled error noise please?

if not incorporating its functionality into the library itself

If that was possible we would have done it :-)

@erikras
Copy link
Author

erikras commented Oct 4, 2017

Still, could you share a minimal project that spits out the unhandled error noise please?

For whom? For the jest issue?

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

Yeah.

@gaearon
Copy link
Collaborator

gaearon commented Nov 28, 2017

I have several tests that need to throw errors to alert library users that they are doing something wrong, but my test logs are now full of bright red

With #11636, there is an escape hatch for this.
Something like:

Error.prototype.suppressReactErrorLogging = true;
try {
  // test that something throws an error  
} finally {
  Error.prototype.suppressReactErrorLogging = false;
}

That should be out in 16.2.

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

No branches or pull requests

2 participants