Skip to content

Commit

Permalink
WIP Downgrade deprecation warnings from errors to warnings
Browse files Browse the repository at this point in the history
**RFC:**
Pushing this for feedback while I fix all the failing tests.

**what is the change?:**
Swapping out `warning` module for a fork that uses `console.warn`.
It looks like we were using the `warning` module for deprecation notices, *but* there is also a 'deprecated' module designed specifically for deprecation notices.

However, we could not find any place that it was currently used.

Since React's build process is not 100% clear to me, I assume it could still be used somewhere by something and just updated it along with other deprecation notices.

We might consider a follow-up diff that does some clean up here;
 - remove 'deprecated' module if it's unused, OR
 - use 'deprecated' module for all our current deprecation warnings

**why make this change?:**
- We have had complaints about noisy warnings, in particular after introducing new deprecations
- They potentially cause CI failures
- Deprecations are not really time-sensitive, can ship without breaking your app, etc.

For more context - facebook#9395

**test plan:**
`npm run test`
and unit tests for the new modules
and manual testing (WIP)

**issue:**
facebook#9395
  • Loading branch information
flarnie committed May 10, 2017
1 parent e249c93 commit d1e2188
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 18 deletions.
12 changes: 6 additions & 6 deletions src/isomorphic/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var createFactory = ReactElement.createFactory;
var cloneElement = ReactElement.cloneElement;

if (__DEV__) {
var warning = require('fbjs/lib/warning');
var lowPriorityWarning = require('lowPriorityWarning');
var canDefineProperty = require('canDefineProperty');
var ReactElementValidator = require('ReactElementValidator');
createElement = ReactElementValidator.createElement;
Expand Down Expand Up @@ -91,7 +91,7 @@ if (__DEV__) {
let warnedForPropTypes = false;

React.createMixin = function(mixin) {
warning(
lowPriorityWarning(
warnedForCreateMixin,
'React.createMixin is deprecated and should not be used. You ' +
'can use this mixin directly instead.',
Expand All @@ -104,7 +104,7 @@ if (__DEV__) {
if (canDefineProperty) {
Object.defineProperty(React, 'checkPropTypes', {
get() {
warning(
lowPriorityWarning(
warnedForCheckPropTypes,
'checkPropTypes has been moved to a separate package. ' +
'Accessing React.checkPropTypes is no longer supported ' +
Expand All @@ -119,7 +119,7 @@ if (__DEV__) {

Object.defineProperty(React, 'createClass', {
get: function() {
warning(
lowPriorityWarning(
warnedForCreateClass,
'React.createClass is no longer supported. Use a plain JavaScript ' +
"class instead. If you're not yet ready to migrate, " +
Expand All @@ -133,7 +133,7 @@ if (__DEV__) {

Object.defineProperty(React, 'PropTypes', {
get() {
warning(
lowPriorityWarning(
warnedForPropTypes,
'PropTypes has been moved to a separate package. ' +
'Accessing React.PropTypes is no longer supported ' +
Expand All @@ -155,7 +155,7 @@ if (__DEV__) {
Object.keys(ReactDOMFactories).forEach(function(factory) {
React.DOM[factory] = function(...args) {
if (!warnedForFactories) {
warning(
lowPriorityWarning(
false,
'Accessing factories like React.DOM.%s has been deprecated ' +
'and will be removed in the future. Use the ' +
Expand Down
4 changes: 2 additions & 2 deletions src/isomorphic/classic/element/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var getIteratorFn = require('getIteratorFn');

if (__DEV__) {
var checkPropTypes = require('prop-types/checkPropTypes');
var warning = require('fbjs/lib/warning');
var lowPriorityWarning = require('lowPriorityWarning');
var ReactDebugCurrentFrame = require('ReactDebugCurrentFrame');
var {getCurrentStackAddendum} = require('ReactComponentTreeHook');
}
Expand Down Expand Up @@ -285,7 +285,7 @@ var ReactElementValidator = {
Object.defineProperty(validatedFactory, 'type', {
enumerable: false,
get: function() {
warning(
lowPriorityWarning(
false,
'Factory.type is deprecated. Access the class directly ' +
'before passing it to createFactory.',
Expand Down
4 changes: 2 additions & 2 deletions src/isomorphic/modern/class/ReactBaseClasses.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ var ReactNoopUpdateQueue = require('ReactNoopUpdateQueue');
var canDefineProperty = require('canDefineProperty');
var emptyObject = require('fbjs/lib/emptyObject');
var invariant = require('fbjs/lib/invariant');
var warning = require('fbjs/lib/warning');
var lowPriorityWarning = require('lowPriorityWarning');

/**
* Base class helpers for the updating state of a component.
Expand Down Expand Up @@ -108,7 +108,7 @@ if (__DEV__) {
if (canDefineProperty) {
Object.defineProperty(ReactComponent.prototype, methodName, {
get: function() {
warning(
lowPriorityWarning(
false,
'%s(...) is deprecated in plain JavaScript React classes. %s',
info[0],
Expand Down
6 changes: 3 additions & 3 deletions src/renderers/shared/ReactPerf.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
'use strict';

var ReactDebugTool = require('ReactDebugTool');
var warning = require('fbjs/lib/warning');
var lowPriorityWarning = require('lowPriorityWarning');
var alreadyWarned = false;

import type {FlushHistory} from 'ReactDebugTool';
Expand Down Expand Up @@ -390,7 +390,7 @@ function printOperations(flushHistory?: FlushHistory) {

var warnedAboutPrintDOM = false;
function printDOM(measurements: FlushHistory) {
warning(
lowPriorityWarning(
warnedAboutPrintDOM,
'`ReactPerf.printDOM(...)` is deprecated. Use ' +
'`ReactPerf.printOperations(...)` instead.',
Expand All @@ -401,7 +401,7 @@ function printDOM(measurements: FlushHistory) {

var warnedAboutGetMeasurementsSummaryMap = false;
function getMeasurementsSummaryMap(measurements: FlushHistory) {
warning(
lowPriorityWarning(
warnedAboutGetMeasurementsSummaryMap,
'`ReactPerf.getMeasurementsSummaryMap(...)` is deprecated. Use ' +
'`ReactPerf.getWasted(...)` instead.',
Expand Down
5 changes: 3 additions & 2 deletions src/renderers/shared/fiber/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const isArray = Array.isArray;
if (__DEV__) {
var {startPhaseTimer, stopPhaseTimer} = require('ReactDebugFiberPerf');
var warning = require('fbjs/lib/warning');
var lowPriorityWarning = requier('lowPriorityWarning');
var warnOnInvalidCallback = function(callback: mixed, callerName: string) {
warning(
callback === null || typeof callback === 'function',
Expand Down Expand Up @@ -316,7 +317,7 @@ module.exports = function(

if (oldState !== instance.state) {
if (__DEV__) {
warning(
lowPriorityWarning(
false,
'%s.componentWillMount(): Assigning directly to this.state is ' +
"deprecated (except inside a component's " +
Expand Down Expand Up @@ -345,7 +346,7 @@ module.exports = function(

if (instance.state !== oldState) {
if (__DEV__) {
warning(
lowPriorityWarning(
false,
'%s.componentWillReceiveProps(): Assigning directly to ' +
"this.state is deprecated (except inside a component's " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ if (__DEV__) {
var checkPropTypes = require('prop-types/checkPropTypes');
var emptyObject = require('fbjs/lib/emptyObject');
var invariant = require('fbjs/lib/invariant');
var lowPriorityWarning = require('lowPriorityWarning');
var shallowEqual = require('fbjs/lib/shallowEqual');
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
var warning = require('fbjs/lib/warning');
Expand Down Expand Up @@ -879,7 +880,7 @@ var ReactCompositeComponent = {
inst.state = beforeState;
inst.updater.enqueueReplaceState(inst, afterState);
if (__DEV__) {
warning(
lowPriorityWarning(
false,
'%s.componentWillReceiveProps(): Assigning directly to ' +
"this.state is deprecated (except inside a component's " +
Expand Down
4 changes: 2 additions & 2 deletions src/shared/utils/deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

'use strict';

var warning = require('fbjs/lib/warning');
var lowPriorityWarning = require('lowPriorityWarning');

/**
* This will log a single deprecation notice per function and forward the call
Expand All @@ -35,7 +35,7 @@ function deprecated<T: Function>(
var warned = false;
if (__DEV__) {
var newFn = function() {
warning(
lowPriorityWarning(
warned,
/* eslint-disable no-useless-concat */
// Require examples in this string must be split to prevent React's
Expand Down
41 changes: 41 additions & 0 deletions src/shared/utils/lowPriorityWarning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* Copyright 2014-2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule lowPriorityWarning
*/

'use strict';

/**
* Forked from fbjs/warning:
* https://github.com/facebook/fbjs/blob/e66ba20ad5be433eb54423f2b097d829324d9de6/packages/fbjs/src/__forks__/warning.js
*
* Only change is we use console.warn instead of console.error,
* and do nothing when 'console' is not supported.
* This really simplifies the code.
* ---
*
* Similar to invariant but only logs a warning if the condition is not met.
* This can be used to log issues in development environments in critical
* paths. Removing the logging code for production environments will keep the
* same logic and follow the same code paths.
*/

var lowPriorityWarning = function() {};

if (__DEV__) {
lowPriorityWarning = function(condition, format, ...args) {
var argIndex = 0;
var message = 'Warning: ' + format.replace(/%s/g, () => args[argIndex++]);
if (typeof console !== 'undefined') {
console.warn(message);
}
};
}

module.exports = lowPriorityWarning;

0 comments on commit d1e2188

Please sign in to comment.