Cherrypick commit to downgrade deprecations to warnings (#9753)

flarnie · web-flow · commit 413c7b548a88 · 2017-05-23T15:11:51.000-07:00
* Downgrade deprecation warnings from errors to warnings (#9650) * Downgrade deprecation warnings from errors to warnings **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 - #9395 **test plan:** `npm run test` and unit tests for the new modules and manual testing (WIP) **issue:** #9395 * Add 'lowPriorityWarning' to ReactExternals **what is the change?:** We won't bundle 'lowPriorityWarning' with the rest of React when building for Facebook. NOTE: A parallel commit will introduce an internal implementation of 'lowPriorityWarning' in Facebook's codebase, to compensate. Will post a comment with the diff number once that is up. **why make this change?:** So that the sync between github and Facebook can go more smoothly! **test plan:** We will see when I run the sync! But this is a reasonable first step imo. **issue:** #9398 * Tweaks to get tests passing after cherry-picking PR#9650 **what is the change?:** - adds 'lowPriorityWarning' for deprecation of '__spread' and 'createMixin' - tweaks test to check for 'warn' and not 'error' **why make this change?:** Both these issues were introduced by merge conflict resolution when cherry-picking this change from master onto 15.6. **test plan:** `yarn test` **issue:** * Fix mis-written 'require' for 'warning' module **what is the change?:** Fixes 'warning' to be required from 'warning' **why make this change?:** It was causing the browserify build to crash, because we don't expect to have a path to 'warning'. **test plan:** CI
diff --git a/src/addons/__tests__/ReactDOMFactories-test.js b/src/addons/__tests__/ReactDOMFactories-test.js
@@ -17,22 +17,24 @@ var {div} = require('ReactDOMFactories');
 describe('ReactDOMFactories', () => {
   it('allow factories to be called without warnings', () => {
     spyOn(console, 'error');
+    spyOn(console, 'warn');
     var element = div();
     expect(element.type).toBe('div');
     expect(console.error).not.toHaveBeenCalled();
+    expect(console.warn).not.toHaveBeenCalled();
   });
 
   it('warns once when accessing React.DOM methods', () => {
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
 
     var a = React.DOM.a();
     var p = React.DOM.p();
 
     expect(a.type).toBe('a');
     expect(p.type).toBe('p');
 
-    expect(console.error).toHaveBeenCalledTimes(1);
-    expect(console.error.calls.first().args[0]).toContain(
+    expect(console.warn).toHaveBeenCalledTimes(1);
+    expect(console.warn.calls.first().args[0]).toContain(
       'Warning: Accessing factories like React.DOM.a has been deprecated',
     );
   });
diff --git a/src/isomorphic/React.js b/src/isomorphic/React.js
@@ -21,13 +21,13 @@ var ReactPropTypes = require('ReactPropTypes');
 var ReactVersion = require('ReactVersion');
 
 var onlyChild = require('onlyChild');
-var warning = require('warning');
 
 var createElement = ReactElement.createElement;
 var createFactory = ReactElement.createFactory;
 var cloneElement = ReactElement.cloneElement;
 
 if (__DEV__) {
+  var lowPriorityWarning = require('lowPriorityWarning');
   var canDefineProperty = require('canDefineProperty');
   var ReactElementValidator = require('ReactElementValidator');
   var didWarnPropTypesDeprecated = false;
@@ -45,7 +45,7 @@ if (__DEV__) {
   var warnedForSpread = false;
   var warnedForCreateMixin = false;
   __spread = function() {
-    warning(
+    lowPriorityWarning(
       warnedForSpread,
       'React.__spread is deprecated and should not be used. Use ' +
         'Object.assign directly or another helper function with similar ' +
@@ -57,7 +57,7 @@ if (__DEV__) {
   };
 
   createMixin = function(mixin) {
-    warning(
+    lowPriorityWarning(
       warnedForCreateMixin,
       'React.createMixin is deprecated and should not be used. You ' +
         'can use this mixin directly instead.',
@@ -107,7 +107,7 @@ if (__DEV__) {
   if (canDefineProperty) {
     Object.defineProperty(React, 'PropTypes', {
       get() {
-        warning(
+        lowPriorityWarning(
           didWarnPropTypesDeprecated,
           'Accessing PropTypes via the main React package is deprecated. Use ' +
             'the prop-types package from npm instead.',
@@ -126,7 +126,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 ' +
diff --git a/src/isomorphic/__tests__/React-test.js b/src/isomorphic/__tests__/React-test.js
@@ -19,21 +19,21 @@ describe('React', () => {
   });
 
   it('should log a deprecation warning once when using React.__spread', () => {
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     React.__spread({});
     React.__spread({});
-    expect(console.error.calls.count()).toBe(1);
-    expect(console.error.calls.argsFor(0)[0]).toContain(
+    expect(console.warn.calls.count()).toBe(1);
+    expect(console.warn.calls.argsFor(0)[0]).toContain(
       'React.__spread is deprecated and should not be used',
     );
   });
 
   it('should log a deprecation warning once when using React.createMixin', () => {
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     React.createMixin();
     React.createMixin();
-    expect(console.error.calls.count()).toBe(1);
-    expect(console.error.calls.argsFor(0)[0]).toContain(
+    expect(console.warn.calls.count()).toBe(1);
+    expect(console.warn.calls.argsFor(0)[0]).toContain(
       'React.createMixin is deprecated and should not be used',
     );
   });
diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js
@@ -27,6 +27,7 @@ var checkReactTypeSpec = require('checkReactTypeSpec');
 var canDefineProperty = require('canDefineProperty');
 var getIteratorFn = require('getIteratorFn');
 var warning = require('warning');
+var lowPriorityWarning = require('lowPriorityWarning');
 
 function getDeclarationErrorAddendum() {
   if (ReactCurrentOwner.current) {
@@ -275,7 +276,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.',
diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
@@ -444,20 +444,20 @@ describe('ReactElementValidator', () => {
   });
 
   it('should warn when accessing .type on an element factory', () => {
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     function TestComponent() {
       return <div />;
     }
     var TestFactory = React.createFactory(TestComponent);
     expect(TestFactory.type).toBe(TestComponent);
-    expect(console.error.calls.count()).toBe(1);
-    expect(console.error.calls.argsFor(0)[0]).toBe(
+    expect(console.warn.calls.count()).toBe(1);
+    expect(console.warn.calls.argsFor(0)[0]).toBe(
       'Warning: Factory.type is deprecated. Access the class directly before ' +
         'passing it to createFactory.',
     );
     // Warn once, not again
     expect(TestFactory.type).toBe(TestComponent);
-    expect(console.error.calls.count()).toBe(1);
+    expect(console.warn.calls.count()).toBe(1);
   });
 
   it('does not warn when using DOM node as children', () => {
diff --git a/src/isomorphic/modern/class/ReactComponent.js b/src/isomorphic/modern/class/ReactComponent.js
@@ -16,7 +16,7 @@ var ReactNoopUpdateQueue = require('ReactNoopUpdateQueue');
 var canDefineProperty = require('canDefineProperty');
 var emptyObject = require('emptyObject');
 var invariant = require('invariant');
-var warning = require('warning');
+var lowPriorityWarning = require('lowPriorityWarning');
 
 /**
  * Base class helpers for the updating state of a component.
@@ -114,7 +114,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],
diff --git a/src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee b/src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee
@@ -378,16 +378,16 @@ describe 'ReactCoffeeScriptClass', ->
     undefined
 
   it 'should throw AND warn when trying to access classic APIs', ->
-    spyOn console, 'error'
+    spyOn console, 'warn'
     instance =
       test Inner(name: 'foo'), 'DIV', 'foo'
     expect(-> instance.replaceState {}).toThrow()
     expect(-> instance.isMounted()).toThrow()
-    expect(console.error.calls.count()).toBe 2
-    expect(console.error.calls.argsFor(0)[0]).toContain(
+    expect(console.warn.calls.count()).toBe 2
+    expect(console.warn.calls.argsFor(0)[0]).toContain(
       'replaceState(...) is deprecated in plain JavaScript React classes'
     )
-    expect(console.error.calls.argsFor(1)[0]).toContain(
+    expect(console.warn.calls.argsFor(1)[0]).toContain(
       'isMounted(...) is deprecated in plain JavaScript React classes'
     )
     undefined
diff --git a/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js b/src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
@@ -408,15 +408,15 @@ describe('ReactES6Class', () => {
   });
 
   it('should throw AND warn when trying to access classic APIs', () => {
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     var instance = test(<Inner name="foo" />, 'DIV', 'foo');
     expect(() => instance.replaceState({})).toThrow();
     expect(() => instance.isMounted()).toThrow();
-    expect(console.error.calls.count()).toBe(2);
-    expect(console.error.calls.argsFor(0)[0]).toContain(
+    expect(console.warn.calls.count()).toBe(2);
+    expect(console.warn.calls.argsFor(0)[0]).toContain(
       'replaceState(...) is deprecated in plain JavaScript React classes',
     );
-    expect(console.error.calls.argsFor(1)[0]).toContain(
+    expect(console.warn.calls.argsFor(1)[0]).toContain(
       'isMounted(...) is deprecated in plain JavaScript React classes',
     );
   });
diff --git a/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts b/src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts
@@ -502,18 +502,18 @@ describe('ReactTypeScriptClass', function() {
   });
 
   it('should throw AND warn when trying to access classic APIs', function() {
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     var instance = test(
       React.createElement(Inner, {name: 'foo'}),
       'DIV','foo'
     );
     expect(() => instance.replaceState({})).toThrow();
     expect(() => instance.isMounted()).toThrow();
-    expect((<any>console.error).calls.count()).toBe(2);
-    expect((<any>console.error).calls.argsFor(0)[0]).toContain(
+    expect((<any>console.warn).calls.count()).toBe(2);
+    expect((<any>console.warn).calls.argsFor(0)[0]).toContain(
       'replaceState(...) is deprecated in plain JavaScript React classes'
     );
-    expect((<any>console.error).calls.argsFor(1)[0]).toContain(
+    expect((<any>console.warn).calls.argsFor(1)[0]).toContain(
       'isMounted(...) is deprecated in plain JavaScript React classes'
     );
   });
diff --git a/src/renderers/dom/client/__tests__/ReactDOM-test.js b/src/renderers/dom/client/__tests__/ReactDOM-test.js
@@ -110,10 +110,10 @@ describe('ReactDOM', () => {
   });
 
   it('throws warning when  React.DOM factories are called', () => {
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     var element = React.DOM.div();
     expect(element.type).toBe('div');
-    expect(console.error.calls.count()).toBe(1);
+    expect(console.warn.calls.count()).toBe(1);
   });
 
   it('throws in render() if the mount callback is not a function', () => {
diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js
@@ -13,7 +13,7 @@
 'use strict';
 
 var ReactDebugTool = require('ReactDebugTool');
-var warning = require('warning');
+var lowPriorityWarning = require('lowPriorityWarning');
 var alreadyWarned = false;
 
 import type {FlushHistory} from 'ReactDebugTool';
@@ -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.',
@@ -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.',
diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js
@@ -401,30 +401,30 @@ describe('ReactPerf', () => {
 
   it('warns once when using getMeasurementsSummaryMap', () => {
     var measurements = measure(() => {});
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     ReactPerf.getMeasurementsSummaryMap(measurements);
-    expect(console.error.calls.count()).toBe(1);
-    expect(console.error.calls.argsFor(0)[0]).toContain(
+    expect(console.warn.calls.count()).toBe(1);
+    expect(console.warn.calls.argsFor(0)[0]).toContain(
       '`ReactPerf.getMeasurementsSummaryMap(...)` is deprecated. Use ' +
         '`ReactPerf.getWasted(...)` instead.',
     );
 
     ReactPerf.getMeasurementsSummaryMap(measurements);
-    expect(console.error.calls.count()).toBe(1);
+    expect(console.warn.calls.count()).toBe(1);
   });
 
   it('warns once when using printDOM', () => {
     var measurements = measure(() => {});
-    spyOn(console, 'error');
+    spyOn(console, 'warn');
     ReactPerf.printDOM(measurements);
-    expect(console.error.calls.count()).toBe(1);
-    expect(console.error.calls.argsFor(0)[0]).toContain(
+    expect(console.warn.calls.count()).toBe(1);
+    expect(console.warn.calls.argsFor(0)[0]).toContain(
       '`ReactPerf.printDOM(...)` is deprecated. Use ' +
         '`ReactPerf.printOperations(...)` instead.',
     );
 
     ReactPerf.printDOM(measurements);
-    expect(console.error.calls.count()).toBe(1);
+    expect(console.warn.calls.count()).toBe(1);
   });
 
   it('returns isRunning state', () => {
diff --git a/src/shared/utils/deprecated.js b/src/shared/utils/deprecated.js
@@ -12,7 +12,7 @@
 
 'use strict';
 
-var warning = require('warning');
+var lowPriorityWarning = require('lowPriorityWarning');
 
 /**
  * This will log a single deprecation notice per function and forward the call
@@ -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
diff --git a/src/shared/utils/lowPriorityWarning.js b/src/shared/utils/lowPriorityWarning.js
@@ -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 (!condition && typeof console !== 'undefined') {
+      console.warn(message);
+    }
+  };
+}
+
+module.exports = lowPriorityWarning;
