Skip to content

Commit 964c263

Browse files
flarnieflarnie
authored
Downgrade deprecation warnings from errors to warnings (#9650)
* Initial regeneration of results.json **what is the change?:** We ran `yarn build` and updated the perf. stats record. **why make this change?:** Some commits have landed without updating this. By getting an initial update, I can run the build script again after my changes and see any size regressions. * 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 * Make state mutations an error, not low-pri warning **what is the change?:** Even though this is a "deprecation" warning, we still want to use 'console.error' for it. **why make this change?:** - It's not likely to come up now, hopefully, because this warning has been present for some time - This will cause real issues in production if ignored **test plan:** `yarn test` - we did fix one test which failed bc of this change **issue:** #9398 * Fix test of assigning to this.state that was only passing in fiber **what is the change?:** updated a unit test for assigning directly to state; it once again raises an error and not a warning. **why make this change?:** So that tests pass **test plan:** REACT_DOM_JEST_USE_FIBER=1 yarn run test **issue:** * Update results.json
1 parent 1f80931 commit 964c263

File tree

15 files changed

+161
-108
lines changed

15 files changed

+161
-108
lines changed

‎scripts/rollup/modules.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ function getExternalModules(externals, bundleType, isRenderer) {
162162
case FB_PROD:
163163
fbjsModules.forEach(module => externalModules.push(module));
164164
externalModules.push('ReactCurrentOwner');
165+
externalModules.push('lowPriorityWarning');
165166
if (isRenderer) {
166167
externalModules.push('React');
167168
if (externalModules.indexOf('react-dom') > -1) {

‎scripts/rollup/results.json‎

Lines changed: 62 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,92 +1,92 @@
11
{
22
"bundleSizes": {
33
"react.development.js (UMD_DEV)": {
4-
"size": 123496,
5-
"gzip": 31034
4+
"size": 125119,
5+
"gzip": 31339
66
},
77
"react.production.min.js (UMD_PROD)": {
88
"size": 15753,
99
"gzip": 5824
1010
},
1111
"react-dom.development.js (UMD_DEV)": {
12-
"size": 592317,
13-
"gzip": 136560
12+
"size": 593408,
13+
"gzip": 136701
1414
},
1515
"react-dom.production.min.js (UMD_PROD)": {
16-
"size": 121979,
17-
"gzip": 38564
16+
"size": 122214,
17+
"gzip": 38682
1818
},
1919
"react-dom-server.development.js (UMD_DEV)": {
20-
"size": 498439,
21-
"gzip": 120624
20+
"size": 525896,
21+
"gzip": 126687
2222
},
2323
"react-dom-server.production.min.js (UMD_PROD)": {
24-
"size": 107169,
25-
"gzip": 33472
24+
"size": 111581,
25+
"gzip": 34959
2626
},
2727
"react-art.development.js (UMD_DEV)": {
28-
"size": 349177,
29-
"gzip": 77902
28+
"size": 349191,
29+
"gzip": 77905
3030
},
3131
"react-art.production.min.js (UMD_PROD)": {
32-
"size": 96267,
33-
"gzip": 29310
32+
"size": 96277,
33+
"gzip": 29313
3434
},
3535
"react.development.js (NODE_DEV)": {
36-
"size": 70293,
37-
"gzip": 17777
36+
"size": 71914,
37+
"gzip": 18276
3838
},
3939
"react.production.min.js (NODE_PROD)": {
4040
"size": 9195,
4141
"gzip": 3614
4242
},
4343
"React-dev.js (FB_DEV)": {
44-
"size": 71576,
45-
"gzip": 18181
44+
"size": 73423,
45+
"gzip": 18751
4646
},
4747
"React-prod.js (FB_PROD)": {
48-
"size": 36456,
49-
"gzip": 9216
48+
"size": 36836,
49+
"gzip": 9248
5050
},
5151
"ReactDOMStack-dev.js (FB_DEV)": {
52-
"size": 494069,
53-
"gzip": 117876
52+
"size": 496986,
53+
"gzip": 118763
5454
},
5555
"ReactDOMStack-prod.js (FB_PROD)": {
56-
"size": 352489,
57-
"gzip": 84575
56+
"size": 353129,
57+
"gzip": 84773
5858
},
5959
"react-dom.development.js (NODE_DEV)": {
60-
"size": 549893,
61-
"gzip": 126464
60+
"size": 550984,
61+
"gzip": 126642
6262
},
6363
"react-dom.production.min.js (NODE_PROD)": {
64-
"size": 118192,
65-
"gzip": 37224
64+
"size": 118427,
65+
"gzip": 37300
6666
},
6767
"ReactDOMFiber-dev.js (FB_DEV)": {
68-
"size": 550815,
69-
"gzip": 126958
68+
"size": 551962,
69+
"gzip": 127141
7070
},
7171
"ReactDOMFiber-prod.js (FB_PROD)": {
72-
"size": 410570,
73-
"gzip": 94005
72+
"size": 411657,
73+
"gzip": 94084
7474
},
7575
"react-dom-server.development.js (NODE_DEV)": {
76-
"size": 447050,
77-
"gzip": 108025
76+
"size": 474459,
77+
"gzip": 114087
7878
},
7979
"react-dom-server.production.min.js (NODE_PROD)": {
80-
"size": 101542,
81-
"gzip": 31424
80+
"size": 105956,
81+
"gzip": 32782
8282
},
8383
"ReactDOMServerStack-dev.js (FB_DEV)": {
84-
"size": 445736,
85-
"gzip": 107878
84+
"size": 455985,
85+
"gzip": 109831
8686
},
8787
"ReactDOMServerStack-prod.js (FB_PROD)": {
88-
"size": 333450,
89-
"gzip": 80260
88+
"size": 334090,
89+
"gzip": 80459
9090
},
9191
"ReactARTStack-dev.js (FB_DEV)": {
9292
"size": 143166,
@@ -97,20 +97,20 @@
9797
"gzip": 23039
9898
},
9999
"react-art.development.js (NODE_DEV)": {
100-
"size": 270501,
101-
"gzip": 57767
100+
"size": 270515,
101+
"gzip": 57772
102102
},
103103
"react-art.production.min.js (NODE_PROD)": {
104-
"size": 57652,
105-
"gzip": 17386
104+
"size": 57662,
105+
"gzip": 17391
106106
},
107107
"ReactARTFiber-dev.js (FB_DEV)": {
108-
"size": 269759,
109-
"gzip": 57590
108+
"size": 269773,
109+
"gzip": 57593
110110
},
111111
"ReactARTFiber-prod.js (FB_PROD)": {
112-
"size": 208039,
113-
"gzip": 43486
112+
"size": 208053,
113+
"gzip": 43488
114114
},
115115
"ReactNativeStack.js (RN)": {
116116
"size": 233993,
@@ -121,20 +121,20 @@
121121
"gzip": 84001
122122
},
123123
"ReactTestRendererFiber-dev.js (FB_DEV)": {
124-
"size": 267261,
125-
"gzip": 56462
124+
"size": 267275,
125+
"gzip": 56465
126126
},
127127
"ReactTestRendererStack-dev.js (FB_DEV)": {
128128
"size": 151770,
129129
"gzip": 34846
130130
},
131131
"react-noop-renderer.development.js (NODE_DEV)": {
132-
"size": 259178,
133-
"gzip": 54431
132+
"size": 259192,
133+
"gzip": 54435
134134
},
135135
"react-test-renderer.development.js (NODE_DEV)": {
136-
"size": 268012,
137-
"gzip": 56651
136+
"size": 268026,
137+
"gzip": 56654
138138
},
139139
"react-dom-test-utils.development.js (NODE_DEV)": {
140140
"size": 52792,
@@ -155,6 +155,14 @@
155155
"ReactShallowRenderer-dev.js (FB_DEV)": {
156156
"size": 8063,
157157
"gzip": 2229
158+
},
159+
"ReactDOMServerStream-dev.js (FB_DEV)": {
160+
"size": 472917,
161+
"gzip": 113925
162+
},
163+
"ReactDOMServerStream-prod.js (FB_PROD)": {
164+
"size": 345920,
165+
"gzip": 83497
158166
}
159167
}
160168
}

‎src/addons/__tests__/ReactDOMFactories-test.js‎

Lines changed: 5 additions & 3 deletions
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‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ var createFactory = ReactElement.createFactory;
2727
var cloneElement = ReactElement.cloneElement;
2828

2929
if (__DEV__) {
30-
var warning = require('fbjs/lib/warning');
30+
var lowPriorityWarning = require('lowPriorityWarning');
3131
var canDefineProperty = require('canDefineProperty');
3232
var ReactElementValidator = require('ReactElementValidator');
3333
createElement = ReactElementValidator.createElement;
@@ -91,7 +91,7 @@ if (__DEV__) {
9191
let warnedForPropTypes = false;
9292

9393
React.createMixin = function(mixin) {
94-
warning(
94+
lowPriorityWarning(
9595
warnedForCreateMixin,
9696
'React.createMixin is deprecated and should not be used. You ' +
9797
'can use this mixin directly instead.',
@@ -104,7 +104,7 @@ if (__DEV__) {
104104
if (canDefineProperty) {
105105
Object.defineProperty(React, 'checkPropTypes', {
106106
get() {
107-
warning(
107+
lowPriorityWarning(
108108
warnedForCheckPropTypes,
109109
'checkPropTypes has been moved to a separate package. ' +
110110
'Accessing React.checkPropTypes is no longer supported ' +
@@ -119,7 +119,7 @@ if (__DEV__) {
119119

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

134134
Object.defineProperty(React, 'PropTypes', {
135135
get() {
136-
warning(
136+
lowPriorityWarning(
137137
warnedForPropTypes,
138138
'PropTypes has been moved to a separate package. ' +
139139
'Accessing React.PropTypes is no longer supported ' +
@@ -155,7 +155,7 @@ if (__DEV__) {
155155
Object.keys(ReactDOMFactories).forEach(function(factory) {
156156
React.DOM[factory] = function(...args) {
157157
if (!warnedForFactories) {
158-
warning(
158+
lowPriorityWarning(
159159
false,
160160
'Accessing factories like React.DOM.%s has been deprecated ' +
161161
'and will be removed in the future. Use the ' +

‎src/isomorphic/__tests__/React-test.js‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,22 @@ describe('React', () => {
1919
});
2020

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

3131
it('should warn once when attempting to access React.createClass', () => {
32-
spyOn(console, 'error');
32+
spyOn(console, 'warn');
3333
let createClass = React.createClass;
3434
createClass = React.createClass;
3535
expect(createClass).not.toBe(undefined);
36-
expectDev(console.error.calls.count()).toBe(1);
37-
expectDev(console.error.calls.argsFor(0)[0]).toContain(
36+
expectDev(console.warn.calls.count()).toBe(1);
37+
expectDev(console.warn.calls.argsFor(0)[0]).toContain(
3838
'React.createClass is no longer supported. Use a plain JavaScript ' +
3939
"class instead. If you're not yet ready to migrate, " +
4040
'create-react-class is available on npm as a drop-in replacement. ' +
@@ -43,12 +43,12 @@ describe('React', () => {
4343
});
4444

4545
it('should warn once when attempting to access React.PropTypes', () => {
46-
spyOn(console, 'error');
46+
spyOn(console, 'warn');
4747
let PropTypes = React.PropTypes;
4848
PropTypes = React.PropTypes;
4949
expect(PropTypes).not.toBe(undefined);
50-
expectDev(console.error.calls.count()).toBe(1);
51-
expectDev(console.error.calls.argsFor(0)[0]).toContain(
50+
expectDev(console.warn.calls.count()).toBe(1);
51+
expectDev(console.warn.calls.argsFor(0)[0]).toContain(
5252
'PropTypes has been moved to a separate package. ' +
5353
'Accessing React.PropTypes is no longer supported ' +
5454
'and will be removed completely in React 16. ' +
@@ -58,12 +58,12 @@ describe('React', () => {
5858
});
5959

6060
it('should warn once when attempting to access React.checkPropTypes', () => {
61-
spyOn(console, 'error');
61+
spyOn(console, 'warn');
6262
let checkPropTypes = React.checkPropTypes;
6363
checkPropTypes = React.checkPropTypes;
6464
expect(checkPropTypes).not.toBe(undefined);
65-
expectDev(console.error.calls.count()).toBe(1);
66-
expectDev(console.error.calls.argsFor(0)[0]).toContain(
65+
expectDev(console.warn.calls.count()).toBe(1);
66+
expectDev(console.warn.calls.argsFor(0)[0]).toContain(
6767
'checkPropTypes has been moved to a separate package. ' +
6868
'Accessing React.checkPropTypes is no longer supported ' +
6969
'and will be removed completely in React 16. ' +

‎src/isomorphic/classic/element/ReactElementValidator.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ var getIteratorFn = require('getIteratorFn');
2727

2828
if (__DEV__) {
2929
var checkPropTypes = require('prop-types/checkPropTypes');
30-
var warning = require('fbjs/lib/warning');
30+
var lowPriorityWarning = require('lowPriorityWarning');
3131
var ReactDebugCurrentFrame = require('ReactDebugCurrentFrame');
32+
var warning = require('fbjs/lib/warning');
3233
var {getCurrentStackAddendum} = require('ReactComponentTreeHook');
3334
}
3435

@@ -285,7 +286,7 @@ var ReactElementValidator = {
285286
Object.defineProperty(validatedFactory, 'type', {
286287
enumerable: false,
287288
get: function() {
288-
warning(
289+
lowPriorityWarning(
289290
false,
290291
'Factory.type is deprecated. Access the class directly ' +
291292
'before passing it to createFactory.',

0 commit comments

Comments
 (0)