React: Warnings should use console.warn

[DennisKo]

Do you want to request a feature or report a bug?
Bug (and Feature?)

What is the current behavior?
Warnings are logged out as console.error()

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/ebsrpraL/).
I am using React 16 RC and the behavior did come up with every warning React logged out so far.

What is the expected behavior?
Warnings should be logged out with console.warn(). This way we can actually filter the output of the console and distinguish between real errors and warnings.

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

[gaearon]

In React we intentionally rely on warnings treated as errors by the user. In fact at Facebook we show a modal dialog for each warning and we may enable the same behaviour in the open source version in the future. Does this help?

[DennisKo]

The problem with that is that many developers do not follow this advice. If you are looking for a fix for your warning you see a lot of issue threads that state something like "Its only a warning in development, just ignore it and it will be fine in production".

We should have more control over the display of warnings at least until everyone is on board with the "treat warning as an error" thinking. Is there something planned to handle warnings ourselves in the future?

[gaearon]

If you are looking for a fix for your warning you see a lot of issue threads that state something like "Its only a warning in development, just ignore it and it will be fine in production".

When a library is big, there will be plenty of bad advice just due to its popularity. I don’t see how we could change this except clarifying our position in threads like this.

We should have more control over the display of warnings at least until everyone is on board with the "treat warning as an error" thinking

There are existing issues about providing more control over warning display (you can search for them). I don’t think we need to get anyone “on board” with our thinking though—this is just how we ask you to use React. You can disagree but please don’t be frustrated if your app doesn’t work correctly. I agree that we should educate users more clearly about this approach though!

[gaearon]

I’ll close this one—as I mentioned before, React asks that you treat warnings as errors. We only emit warnings for things that actually need to be fixed in your code. Ignoring them is dangerous.

For configurable warnings, please track https://github.com/facebook/react/issues/6683 and https://github.com/facebook/react/issues/9189 and related issues linked from them.

[vemv]

Note that e.g. PhantomJS interprets console.error as actual errors (which is reasonable behavior - .error means "an error happened"), so unit tests can fail due to React warnings.

From the linked "configurable warnings" issues, I see that months or even years have passed without any progress in that front.

How much effort does a logging setter (or if/else - whatever) require?

It's not particularly fair that I have to silence every React warning given the FB team went against conventional wisdom (console.warn) _and_ made it non-configurable.

Can you spend 2h on this so developers around the world save 1000s?

Cheers - Victor

[gaearon]

so unit tests can fail due to React warnings.

This sounds like desirable behavior. We don't emit warnings for things that are safe. If you see a warning from React it means there is a bug in your code that you need to fix.

In the past we've used console.error for deprecation warnings too. But we've listened to the feedback and stopped doing that. Because deprecation warnings don't actually mean bugs in your code and shouldn't break tests.

I hope this rationale makes sense.

[gaearon]

Can you spend 2h on this so developers around the world save 1000s?

Warnings don't exist to waste your time, they are there to help you write bug-free code. If some particular warning is very noisy it means we did a poor job of choosing when to fire it.

Can you give an example of a warning that causes much pain for you? Perhaps we're emitting it in wrong cases or don't deduplicate it?

Instead of silencing warnings let's solve the root of the problem, and make them valuable (if they for some reason aren't).

[vemv]

Hi @gaearon , thanks for the answer!

When I see:

Warning: There is an internal error in the React performance measurement code.

I can see that I did something wrong (normally one's real bugs trigger this particular warning). But why is that a warning? It should be an error. Even the phrase itself says it:

_There is an internal error_

On the other hand, this other specimen is different:

Each child in an array or iterator should have a unique "key" prop.

This doesn't mean there's a bug in my code. It's just something subject to improvement -and prioritization-, AFAICT.

So, under the 'warning' umbrella there are both errors and non-errors.

Correct categorization of issues seems a difficult, highly subjective task. Meanwhile React could give developers freedom to choose how to spend their time.

Perhaps the dangerously prefix would help.

React.dangerouslySetLogger(my_logger);

Cheers - Victor

[gaearon]

Regarding

Warning: There is an internal error in the React performance measurement code.

This one was due to my mistake and is not actionable. We shouldn't have printed it in the first place. React 16 doesn't have it.

Each child in an array or iterator should have a unique "key" prop.

While it is not a strict error, it often does indicate bugs. You must specify keys in React. Otherwise you will have very subtle problems when reordering lists, for example.

At Facebook we show those warnings in a modal window so that developers have to fix them. We might eventually start doing this in the open source React as well. But please do treat it as important today.

[JeyDotC]

For God's sake, have you ever heard about EXTERNAL DEPENDENCIES I DON'T HAVE CONTROL OF?

I have, and I mean, I HAVE to work with a module which code is inaccessible to me, so I have no control over it, that means I CAN'T solve the warnings it has, and it throws so many of them (like 200 by a single click) that actual errors/warnings under my control get totally hidden, neglecting this whole crap of warnings as errors.

So, you better provide a way to tune warnings being logged to the browser or many people will be using these crappy things:

global.__old_console_warn = global.__old_console_warn || console.warn;

const ignoredWarnings = [
    'Require cycle:',
    'Warning: isMounted(...) is deprecated',
    'Module RCTImageLoader'
];

global.console.warn = str => {
    const tst = (str || '') + '';
    const ignoreWarning = ignoredWarnings.reduce((acc, message) => !!(acc || tst.includes(message)), false);
    if (ignoreWarning) return undefined;
    return global.__old_console_warn.apply(console, [str]);
};

[JeyDotC]

Ah, never mind, I ended up creating this module:

global.__old_console_error = global.__old_console_error || console.error;

const fallbackTune = { condition: () => true, logFunction: 'error' };

let tunes = [];

function tune(condition, logFunction) {
    tunes.push({condition, logFunction });
}

function customError() {
    const argumentArray = [].slice.call(arguments);
    const currentTune = tunes.find(t => t.condition.apply(t, argumentArray)) || fallbackTune;

    if(currentTune.logFunction === 'error'){
        return global.__old_console_error.apply(console, argumentArray);
    }

    if(['warn', 'warning'].includes(currentTune.logFunction)){
        return console.warn.apply(console, argumentArray);
    }

    if(currentTune.logFunction === 'log'){
        return console.log.apply(console, argumentArray);
    }

    if(currentTune.logFunction === 'trace'){
        return console.log.apply(console, argumentArray);
    }

    if(currentTune.logFunction === 'info'){
        return console.log.apply(console, argumentArray);
    }

    return currentTune.logFunction.apply(currentTune, argumentArray);
}

global.console.error = customError;

export default tune;