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

React warnings should use console.warn and allow verbosity override. #17429

Closed
cchamberlain opened this issue Nov 21, 2019 · 6 comments
Closed

Comments

@cchamberlain
Copy link

Do you want to request a feature or report a bug?
Request a feature (though reporting warnings via console.error has always struck me as odd).

What is the current behavior?
After React upgrades and intermittently during development React deprecation and other warnings accrue and React displays them along with the entire call stack via console.error. We eventually pay down the debt by updating all the call points but this isn't something seen as very high priority (actual bugs / certain features / genuine errors are priority). In the meantime the chrome dev console is inundated with warnings masquerading as errors.

image

What is the expected behavior?
These should be appropriately reported via console.warn. This is the standard used for deprecation warnings in chrome. Reporting these as errors makes legitimate errors more difficult to spot, especially due to the fact that the entire call stack is dropped into the message. With the stack trace in the message, each one takes the entire screen of chrome dev tools, where legitimate errors correctly fold the stack trace and take a fraction of the real estate. Reporting warnings via console.error when no error is thrown is not a good practice. It feels a bit like React is yelling at me in all caps to "FIX ME NOW" and clearly thinks its more important than other things (like actual errors). Further, reporting as errors breaks the functionality of chrome dev tools verbosity filters in that I can't focus on actual errors when I want to, nor can I focus on sitewide warnings (including React).

There are times when reporting these as errors would be useful - when I'm doing dedicated work to fix React deprecations. I separate work into separate PRs and often do not want to fix one-off deprecations when I'm working on something unrelated. Often, these warnings cannot be fixed without testing them thoroughly (anything with setState / componentWillMount / etc.).

I propose allowing some configuration such as process.env.REACT_THROW_WARNINGS / process.env.REACT_TREAT_WARNINGS_AS_ERRORS / process.env.REACT_STRICT (or possibly even allowing a verbosity level to report them at). If this flag were enabled, the warnings should actually be thrown, breaking the app, and the call stack would not need to be appended to the message. The default state should report the warning with console.warn and elide the call stack. The warning message could contain a note / link describing how to elevate warnings to actual errors.

If all the above is deemed "by design" or whatever, it would be really nice if React could expose a warning formatter middleware to let us override the massive call stack to cap its head to 3-5 lines.

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

@cchamberlain
Copy link
Author

I'm sure this has been filed before, but I think it needs more attention than a +1 on a closed thread.

@bvaughn
Copy link
Contributor

bvaughn commented Nov 26, 2019

Seems like Dan's proposal in #16753 should address this. I don't think we need a separate issue for it. Let's just add the above suggestion as feedback to Dan's RFC.

@jwalkerinterpres
Copy link

This has not been addressed in over half a decade. Please re-open: warnings should be logged at a warning level.

@rickhanlonii
Copy link
Member

The warnings are for actual bugs, so they should be treated as errors. One improvement we made in React 19 is to make recoverable errors report to console.warn. We're not going to support customizing the error behavior because it should not be easier to silence errors that cause bugs.

@jwalkerinterpres
Copy link

Then why not simply remove the word "Warning" from them?

@rickhanlonii
Copy link
Member

We would like to - there's some legacy reasons why we can't but eventually we might.

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

4 participants