Skip to content

Commit b0b592c

Browse files
committed
Cherrypick commit to downgrade deprecations to warnings (facebook#9753)
* Downgrade deprecation warnings from errors to warnings (facebook#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 - facebook#9395 **test plan:** `npm run test` and unit tests for the new modules and manual testing (WIP) **issue:** facebook#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:** facebook#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
1 parent d8a2381 commit b0b592c

File tree

14 files changed

+92
-48
lines changed

14 files changed

+92
-48
lines changed

src/addons/__tests__/ReactDOMFactories-test.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,24 @@ var {div} = require('ReactDOMFactories');
1717
describe('ReactDOMFactories', () => {
1818
it('allow factories to be called without warnings', () => {
1919
spyOn(console, 'error');
20+
spyOn(console, 'warn');
2021
var element = div();
2122
expect(element.type).toBe('div');
2223
expect(console.error).not.toHaveBeenCalled();
24+
expect(console.warn).not.toHaveBeenCalled();
2325
});
2426

2527
it('warns once when accessing React.DOM methods', () => {
26-
spyOn(console, 'error');
28+
spyOn(console, 'warn');
2729

2830
var a = React.DOM.a();
2931
var p = React.DOM.p();
3032

3133
expect(a.type).toBe('a');
3234
expect(p.type).toBe('p');
3335

34-
expect(console.error).toHaveBeenCalledTimes(1);
35-
expect(console.error.calls.first().args[0]).toContain(
36+
expect(console.warn).toHaveBeenCalledTimes(1);
37+
expect(console.warn.calls.first().args[0]).toContain(
3638
'Warning: Accessing factories like React.DOM.a has been deprecated',
3739
);
3840
});

src/isomorphic/React.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ var ReactPropTypes = require('ReactPropTypes');
2121
var ReactVersion = require('ReactVersion');
2222

2323
var onlyChild = require('onlyChild');
24-
var warning = require('warning');
2524

2625
var createElement = ReactElement.createElement;
2726
var createFactory = ReactElement.createFactory;
2827
var cloneElement = ReactElement.cloneElement;
2928

3029
if (__DEV__) {
30+
var lowPriorityWarning = require('lowPriorityWarning');
3131
var canDefineProperty = require('canDefineProperty');
3232
var ReactElementValidator = require('ReactElementValidator');
3333
var didWarnPropTypesDeprecated = false;
@@ -45,7 +45,7 @@ if (__DEV__) {
4545
var warnedForSpread = false;
4646
var warnedForCreateMixin = false;
4747
__spread = function() {
48-
warning(
48+
lowPriorityWarning(
4949
warnedForSpread,
5050
'React.__spread is deprecated and should not be used. Use ' +
5151
'Object.assign directly or another helper function with similar ' +
@@ -57,7 +57,7 @@ if (__DEV__) {
5757
};
5858

5959
createMixin = function(mixin) {
60-
warning(
60+
lowPriorityWarning(
6161
warnedForCreateMixin,
6262
'React.createMixin is deprecated and should not be used. You ' +
6363
'can use this mixin directly instead.',
@@ -107,7 +107,7 @@ if (__DEV__) {
107107
if (canDefineProperty) {
108108
Object.defineProperty(React, 'PropTypes', {
109109
get() {
110-
warning(
110+
lowPriorityWarning(
111111
didWarnPropTypesDeprecated,
112112
'Accessing PropTypes via the main React package is deprecated. Use ' +
113113
'the prop-types package from npm instead.',
@@ -126,7 +126,7 @@ if (__DEV__) {
126126
Object.keys(ReactDOMFactories).forEach(function(factory) {
127127
React.DOM[factory] = function(...args) {
128128
if (!warnedForFactories) {
129-
warning(
129+
lowPriorityWarning(
130130
false,
131131
'Accessing factories like React.DOM.%s has been deprecated ' +
132132
'and will be removed in the future. Use the ' +

src/isomorphic/__tests__/React-test.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,21 @@ describe('React', () => {
1919
});
2020

2121
it('should log a deprecation warning once when using React.__spread', () => {
22-
spyOn(console, 'error');
22+
spyOn(console, 'warn');
2323
React.__spread({});
2424
React.__spread({});
25-
expect(console.error.calls.count()).toBe(1);
26-
expect(console.error.calls.argsFor(0)[0]).toContain(
25+
expect(console.warn.calls.count()).toBe(1);
26+
expect(console.warn.calls.argsFor(0)[0]).toContain(
2727
'React.__spread is deprecated and should not be used',
2828
);
2929
});
3030

3131
it('should log a deprecation warning once when using React.createMixin', () => {
32-
spyOn(console, 'error');
32+
spyOn(console, 'warn');
3333
React.createMixin();
3434
React.createMixin();
35-
expect(console.error.calls.count()).toBe(1);
36-
expect(console.error.calls.argsFor(0)[0]).toContain(
35+
expect(console.warn.calls.count()).toBe(1);
36+
expect(console.warn.calls.argsFor(0)[0]).toContain(
3737
'React.createMixin is deprecated and should not be used',
3838
);
3939
});

src/isomorphic/classic/element/ReactElementValidator.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ var checkReactTypeSpec = require('checkReactTypeSpec');
2727
var canDefineProperty = require('canDefineProperty');
2828
var getIteratorFn = require('getIteratorFn');
2929
var warning = require('warning');
30+
var lowPriorityWarning = require('lowPriorityWarning');
3031

3132
function getDeclarationErrorAddendum() {
3233
if (ReactCurrentOwner.current) {
@@ -275,7 +276,7 @@ var ReactElementValidator = {
275276
Object.defineProperty(validatedFactory, 'type', {
276277
enumerable: false,
277278
get: function() {
278-
warning(
279+
lowPriorityWarning(
279280
false,
280281
'Factory.type is deprecated. Access the class directly ' +
281282
'before passing it to createFactory.',

src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -444,20 +444,20 @@ describe('ReactElementValidator', () => {
444444
});
445445

446446
it('should warn when accessing .type on an element factory', () => {
447-
spyOn(console, 'error');
447+
spyOn(console, 'warn');
448448
function TestComponent() {
449449
return <div />;
450450
}
451451
var TestFactory = React.createFactory(TestComponent);
452452
expect(TestFactory.type).toBe(TestComponent);
453-
expect(console.error.calls.count()).toBe(1);
454-
expect(console.error.calls.argsFor(0)[0]).toBe(
453+
expect(console.warn.calls.count()).toBe(1);
454+
expect(console.warn.calls.argsFor(0)[0]).toBe(
455455
'Warning: Factory.type is deprecated. Access the class directly before ' +
456456
'passing it to createFactory.',
457457
);
458458
// Warn once, not again
459459
expect(TestFactory.type).toBe(TestComponent);
460-
expect(console.error.calls.count()).toBe(1);
460+
expect(console.warn.calls.count()).toBe(1);
461461
});
462462

463463
it('does not warn when using DOM node as children', () => {

src/isomorphic/modern/class/ReactComponent.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ var ReactNoopUpdateQueue = require('ReactNoopUpdateQueue');
1616
var canDefineProperty = require('canDefineProperty');
1717
var emptyObject = require('emptyObject');
1818
var invariant = require('invariant');
19-
var warning = require('warning');
19+
var lowPriorityWarning = require('lowPriorityWarning');
2020

2121
/**
2222
* Base class helpers for the updating state of a component.
@@ -114,7 +114,7 @@ if (__DEV__) {
114114
if (canDefineProperty) {
115115
Object.defineProperty(ReactComponent.prototype, methodName, {
116116
get: function() {
117-
warning(
117+
lowPriorityWarning(
118118
false,
119119
'%s(...) is deprecated in plain JavaScript React classes. %s',
120120
info[0],

src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee

+4-4
Original file line numberDiff line numberDiff line change
@@ -378,16 +378,16 @@ describe 'ReactCoffeeScriptClass', ->
378378
undefined
379379

380380
it 'should throw AND warn when trying to access classic APIs', ->
381-
spyOn console, 'error'
381+
spyOn console, 'warn'
382382
instance =
383383
test Inner(name: 'foo'), 'DIV', 'foo'
384384
expect(-> instance.replaceState {}).toThrow()
385385
expect(-> instance.isMounted()).toThrow()
386-
expect(console.error.calls.count()).toBe 2
387-
expect(console.error.calls.argsFor(0)[0]).toContain(
386+
expect(console.warn.calls.count()).toBe 2
387+
expect(console.warn.calls.argsFor(0)[0]).toContain(
388388
'replaceState(...) is deprecated in plain JavaScript React classes'
389389
)
390-
expect(console.error.calls.argsFor(1)[0]).toContain(
390+
expect(console.warn.calls.argsFor(1)[0]).toContain(
391391
'isMounted(...) is deprecated in plain JavaScript React classes'
392392
)
393393
undefined

src/isomorphic/modern/class/__tests__/ReactES6Class-test.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -408,15 +408,15 @@ describe('ReactES6Class', () => {
408408
});
409409

410410
it('should throw AND warn when trying to access classic APIs', () => {
411-
spyOn(console, 'error');
411+
spyOn(console, 'warn');
412412
var instance = test(<Inner name="foo" />, 'DIV', 'foo');
413413
expect(() => instance.replaceState({})).toThrow();
414414
expect(() => instance.isMounted()).toThrow();
415-
expect(console.error.calls.count()).toBe(2);
416-
expect(console.error.calls.argsFor(0)[0]).toContain(
415+
expect(console.warn.calls.count()).toBe(2);
416+
expect(console.warn.calls.argsFor(0)[0]).toContain(
417417
'replaceState(...) is deprecated in plain JavaScript React classes',
418418
);
419-
expect(console.error.calls.argsFor(1)[0]).toContain(
419+
expect(console.warn.calls.argsFor(1)[0]).toContain(
420420
'isMounted(...) is deprecated in plain JavaScript React classes',
421421
);
422422
});

src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -502,18 +502,18 @@ describe('ReactTypeScriptClass', function() {
502502
});
503503

504504
it('should throw AND warn when trying to access classic APIs', function() {
505-
spyOn(console, 'error');
505+
spyOn(console, 'warn');
506506
var instance = test(
507507
React.createElement(Inner, {name: 'foo'}),
508508
'DIV','foo'
509509
);
510510
expect(() => instance.replaceState({})).toThrow();
511511
expect(() => instance.isMounted()).toThrow();
512-
expect((<any>console.error).calls.count()).toBe(2);
513-
expect((<any>console.error).calls.argsFor(0)[0]).toContain(
512+
expect((<any>console.warn).calls.count()).toBe(2);
513+
expect((<any>console.warn).calls.argsFor(0)[0]).toContain(
514514
'replaceState(...) is deprecated in plain JavaScript React classes'
515515
);
516-
expect((<any>console.error).calls.argsFor(1)[0]).toContain(
516+
expect((<any>console.warn).calls.argsFor(1)[0]).toContain(
517517
'isMounted(...) is deprecated in plain JavaScript React classes'
518518
);
519519
});

src/renderers/dom/client/__tests__/ReactDOM-test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,10 @@ describe('ReactDOM', () => {
110110
});
111111

112112
it('throws warning when React.DOM factories are called', () => {
113-
spyOn(console, 'error');
113+
spyOn(console, 'warn');
114114
var element = React.DOM.div();
115115
expect(element.type).toBe('div');
116-
expect(console.error.calls.count()).toBe(1);
116+
expect(console.warn.calls.count()).toBe(1);
117117
});
118118

119119
it('throws in render() if the mount callback is not a function', () => {

src/renderers/shared/ReactPerf.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
'use strict';
1414

1515
var ReactDebugTool = require('ReactDebugTool');
16-
var warning = require('warning');
16+
var lowPriorityWarning = require('lowPriorityWarning');
1717
var alreadyWarned = false;
1818

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

391391
var warnedAboutPrintDOM = false;
392392
function printDOM(measurements: FlushHistory) {
393-
warning(
393+
lowPriorityWarning(
394394
warnedAboutPrintDOM,
395395
'`ReactPerf.printDOM(...)` is deprecated. Use ' +
396396
'`ReactPerf.printOperations(...)` instead.',
@@ -401,7 +401,7 @@ function printDOM(measurements: FlushHistory) {
401401

402402
var warnedAboutGetMeasurementsSummaryMap = false;
403403
function getMeasurementsSummaryMap(measurements: FlushHistory) {
404-
warning(
404+
lowPriorityWarning(
405405
warnedAboutGetMeasurementsSummaryMap,
406406
'`ReactPerf.getMeasurementsSummaryMap(...)` is deprecated. Use ' +
407407
'`ReactPerf.getWasted(...)` instead.',

src/renderers/shared/__tests__/ReactPerf-test.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -401,30 +401,30 @@ describe('ReactPerf', () => {
401401

402402
it('warns once when using getMeasurementsSummaryMap', () => {
403403
var measurements = measure(() => {});
404-
spyOn(console, 'error');
404+
spyOn(console, 'warn');
405405
ReactPerf.getMeasurementsSummaryMap(measurements);
406-
expect(console.error.calls.count()).toBe(1);
407-
expect(console.error.calls.argsFor(0)[0]).toContain(
406+
expect(console.warn.calls.count()).toBe(1);
407+
expect(console.warn.calls.argsFor(0)[0]).toContain(
408408
'`ReactPerf.getMeasurementsSummaryMap(...)` is deprecated. Use ' +
409409
'`ReactPerf.getWasted(...)` instead.',
410410
);
411411

412412
ReactPerf.getMeasurementsSummaryMap(measurements);
413-
expect(console.error.calls.count()).toBe(1);
413+
expect(console.warn.calls.count()).toBe(1);
414414
});
415415

416416
it('warns once when using printDOM', () => {
417417
var measurements = measure(() => {});
418-
spyOn(console, 'error');
418+
spyOn(console, 'warn');
419419
ReactPerf.printDOM(measurements);
420-
expect(console.error.calls.count()).toBe(1);
421-
expect(console.error.calls.argsFor(0)[0]).toContain(
420+
expect(console.warn.calls.count()).toBe(1);
421+
expect(console.warn.calls.argsFor(0)[0]).toContain(
422422
'`ReactPerf.printDOM(...)` is deprecated. Use ' +
423423
'`ReactPerf.printOperations(...)` instead.',
424424
);
425425

426426
ReactPerf.printDOM(measurements);
427-
expect(console.error.calls.count()).toBe(1);
427+
expect(console.warn.calls.count()).toBe(1);
428428
});
429429

430430
it('returns isRunning state', () => {

src/shared/utils/deprecated.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
'use strict';
1414

15-
var warning = require('warning');
15+
var lowPriorityWarning = require('lowPriorityWarning');
1616

1717
/**
1818
* This will log a single deprecation notice per function and forward the call
@@ -35,7 +35,7 @@ function deprecated<T: Function>(
3535
var warned = false;
3636
if (__DEV__) {
3737
var newFn = function() {
38-
warning(
38+
lowPriorityWarning(
3939
warned,
4040
/* eslint-disable no-useless-concat */
4141
// Require examples in this string must be split to prevent React's
+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* Copyright 2014-2015, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
* @providesModule lowPriorityWarning
10+
*/
11+
12+
'use strict';
13+
14+
/**
15+
* Forked from fbjs/warning:
16+
* https://github.com/facebook/fbjs/blob/e66ba20ad5be433eb54423f2b097d829324d9de6/packages/fbjs/src/__forks__/warning.js
17+
*
18+
* Only change is we use console.warn instead of console.error,
19+
* and do nothing when 'console' is not supported.
20+
* This really simplifies the code.
21+
* ---
22+
*
23+
* Similar to invariant but only logs a warning if the condition is not met.
24+
* This can be used to log issues in development environments in critical
25+
* paths. Removing the logging code for production environments will keep the
26+
* same logic and follow the same code paths.
27+
*/
28+
29+
var lowPriorityWarning = function() {};
30+
31+
if (__DEV__) {
32+
lowPriorityWarning = function(condition, format, ...args) {
33+
var argIndex = 0;
34+
var message = 'Warning: ' + format.replace(/%s/g, () => args[argIndex++]);
35+
if (!condition && typeof console !== 'undefined') {
36+
console.warn(message);
37+
}
38+
};
39+
}
40+
41+
module.exports = lowPriorityWarning;

0 commit comments

Comments
 (0)