Warning system refactoring (part 1) (#16799)
* Rename lowPriorityWarning to lowPriorityWarningWithoutStack This maintains parity with the other warning-like functions. * Duplicate the toWarnDev tests to test toLowPriorityWarnDev * Make a lowPriorityWarning version of warning.js * Extract both variants in print-warning Avoids parsing lowPriorityWarning.js itself as the way it forwards the call to lowPriorityWarningWithoutStack is not analyzable.
Jessica Franco committed
Sep 24, 2019 at 21:45 UTC
18d2e0c03e4496a824fdb7f89ea2a3d60c30d49a
13 files changed
+291
-50
packages/react-dom/src/client/ReactDOM.js
+3
-3
@@ -61,7 +61,7 @@ import ReactVersion from 'shared/ReactVersion';
61
import ReactSharedInternals from 'shared/ReactSharedInternals';
62
import getComponentName from 'shared/getComponentName';
63
import invariant from 'shared/invariant';
64
-import lowPriorityWarning from 'shared/lowPriorityWarning';
64
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
65
import warningWithoutStack from 'shared/warningWithoutStack';
66
import {enableStableConcurrentModeAPIs} from 'shared/ReactFeatureFlags';
67
@@ -542,7 +542,7 @@ function legacyCreateRootFromDOMContainer(
542
if (__DEV__) {
543
if (shouldHydrate && !forceHydrate && !warnedAboutHydrateAPI) {
544
warnedAboutHydrateAPI = true;
545
- lowPriorityWarning(
545
+ lowPriorityWarningWithoutStack(
546
false,
547
'render(): Calling ReactDOM.render() to hydrate server-rendered markup ' +
548
'will stop working in React v17. Replace the ReactDOM.render() call ' +
@@ -801,7 +801,7 @@ const ReactDOM: Object = {
801
unstable_createPortal(...args) {
802
if (!didWarnAboutUnstableCreatePortal) {
803
didWarnAboutUnstableCreatePortal = true;
804
- lowPriorityWarning(
804
+ lowPriorityWarningWithoutStack(
805
false,
806
'The ReactDOM.unstable_createPortal() alias has been deprecated, ' +
807
'and will be removed in React 17+. Update your code to use ' +
packages/react-dom/src/server/ReactPartialRenderer.js
+2
-2
@@ -15,7 +15,7 @@ import type {ReactProvider, ReactContext} from 'shared/ReactTypes';
15
import React from 'react';
16
import invariant from 'shared/invariant';
17
import getComponentName from 'shared/getComponentName';
18
-import lowPriorityWarning from 'shared/lowPriorityWarning';
18
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
19
import warning from 'shared/warning';
20
import warningWithoutStack from 'shared/warningWithoutStack';
21
import describeComponentFrame from 'shared/describeComponentFrame';
@@ -588,7 +588,7 @@ function resolve(
588
const componentName = getComponentName(Component) || 'Unknown';
589
590
if (!didWarnAboutDeprecatedWillMount[componentName]) {
591
- lowPriorityWarning(
591
+ lowPriorityWarningWithoutStack(
592
false,
593
// keep this warning in sync with ReactStrictModeWarning.js
594
'componentWillMount has been renamed, and is not recommended for use. ' +
packages/react-dom/src/test-utils/ReactTestUtils.js
+2
-2
@@ -17,7 +17,7 @@ import {
17
} from 'shared/ReactWorkTags';
18
import SyntheticEvent from 'legacy-events/SyntheticEvent';
19
import invariant from 'shared/invariant';
20
-import lowPriorityWarning from 'shared/lowPriorityWarning';
20
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
21
import {ELEMENT_NODE} from '../shared/HTMLNodeType';
22
import * as DOMTopLevelEventTypes from '../events/DOMTopLevelEventTypes';
23
import {PLUGIN_EVENT_SYSTEM} from 'legacy-events/EventSystemFlags';
@@ -361,7 +361,7 @@ const ReactTestUtils = {
361
mockComponent: function(module, mockTagName) {
362
if (!hasWarnedAboutDeprecatedMockComponent) {
363
hasWarnedAboutDeprecatedMockComponent = true;
364
- lowPriorityWarning(
364
+ lowPriorityWarningWithoutStack(
365
false,
366
'ReactTestUtils.mockComponent() is deprecated. ' +
367
'Use shallow rendering or jest.mock() instead.\n\n' +
packages/react-is/src/ReactIs.js
+2
-2
@@ -25,7 +25,7 @@ import {
25
REACT_SUSPENSE_TYPE,
26
} from 'shared/ReactSymbols';
27
import isValidElementType from 'shared/isValidElementType';
28
-import lowPriorityWarning from 'shared/lowPriorityWarning';
28
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
29
30
export function typeOf(object: any) {
31
if (typeof object === 'object' && object !== null) {
@@ -88,7 +88,7 @@ export function isAsyncMode(object: any) {
88
if (__DEV__) {
89
if (!hasWarnedAboutDeprecatedIsAsyncMode) {
90
hasWarnedAboutDeprecatedIsAsyncMode = true;
91
- lowPriorityWarning(
91
+ lowPriorityWarningWithoutStack(
92
false,
93
'The ReactIs.isAsyncMode() alias has been deprecated, ' +
94
'and will be removed in React 17+. Update your code to use ' +
packages/react-reconciler/src/ReactStrictModeWarnings.js
+4
-4
@@ -13,7 +13,7 @@ import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
13
14
import getComponentName from 'shared/getComponentName';
15
import {StrictMode} from './ReactTypeOfMode';
16
-import lowPriorityWarning from 'shared/lowPriorityWarning';
16
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
17
import warningWithoutStack from 'shared/warningWithoutStack';
18
19
type FiberArray = Array<Fiber>;
@@ -237,7 +237,7 @@ if (__DEV__) {
237
if (componentWillMountUniqueNames.size > 0) {
238
const sortedNames = setToSortedString(componentWillMountUniqueNames);
239
240
- lowPriorityWarning(
240
+ lowPriorityWarningWithoutStack(
241
false,
242
'componentWillMount has been renamed, and is not recommended for use. ' +
243
'See https://fb.me/react-unsafe-component-lifecycles for details.\n\n' +
@@ -256,7 +256,7 @@ if (__DEV__) {
256
componentWillReceivePropsUniqueNames,
257
);
258
259
- lowPriorityWarning(
259
+ lowPriorityWarningWithoutStack(
260
false,
261
'componentWillReceiveProps has been renamed, and is not recommended for use. ' +
262
'See https://fb.me/react-unsafe-component-lifecycles for details.\n\n' +
@@ -276,7 +276,7 @@ if (__DEV__) {
276
if (componentWillUpdateUniqueNames.size > 0) {
277
const sortedNames = setToSortedString(componentWillUpdateUniqueNames);
278
279
- lowPriorityWarning(
279
+ lowPriorityWarningWithoutStack(
280
false,
281
'componentWillUpdate has been renamed, and is not recommended for use. ' +
282
'See https://fb.me/react-unsafe-component-lifecycles for details.\n\n' +
packages/react/src/ReactBaseClasses.js
+2
-2
@@ -6,7 +6,7 @@
6
*/
7
8
import invariant from 'shared/invariant';
9
-import lowPriorityWarning from 'shared/lowPriorityWarning';
9
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
10
11
import ReactNoopUpdateQueue from './ReactNoopUpdateQueue';
12
@@ -105,7 +105,7 @@ if (__DEV__) {
105
const defineDeprecationWarning = function(methodName, info) {
106
Object.defineProperty(Component.prototype, methodName, {
107
get: function() {
108
- lowPriorityWarning(
108
+ lowPriorityWarningWithoutStack(
109
false,
110
'%s(...) is deprecated in plain JavaScript React classes. %s',
111
info[0],
packages/react/src/ReactElementValidator.js
+2
-2
@@ -12,7 +12,7 @@
12
* that support it.
13
*/
14
15
-import lowPriorityWarning from 'shared/lowPriorityWarning';
15
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
16
import isValidElementType from 'shared/isValidElementType';
17
import getComponentName from 'shared/getComponentName';
18
import {
@@ -477,7 +477,7 @@ export function createFactoryWithValidation(type) {
477
Object.defineProperty(validatedFactory, 'type', {
478
enumerable: false,
479
get: function() {
480
- lowPriorityWarning(
480
+ lowPriorityWarningWithoutStack(
481
false,
482
'Factory.type is deprecated. Access the class directly ' +
483
'before passing it to createFactory.',
packages/shared/forks/lowPriorityWarningWithoutStack.www.js
renamed
+1
@@ -5,4 +5,5 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
+// This "lowPriorityWarning" is an external module
9
export default require('lowPriorityWarning');
packages/shared/lowPriorityWarning.js
+10
-30
@@ -5,47 +5,27 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
+import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
9
+import ReactSharedInternals from 'shared/ReactSharedInternals';
10
+
11
/**
9
- * Forked from fbjs/warning:
10
- * https://github.com/facebook/fbjs/blob/e66ba20ad5be433eb54423f2b097d829324d9de6/packages/fbjs/src/__forks__/warning.js
11
- *
12
- * Only change is we use console.warn instead of console.error,
13
- * and do nothing when 'console' is not supported.
14
- * This really simplifies the code.
15
- * ---
12
* Similar to invariant but only logs a warning if the condition is not met.
13
* This can be used to log issues in development environments in critical
14
* paths. Removing the logging code for production environments will keep the
15
* same logic and follow the same code paths.
16
*/
17
22
-let lowPriorityWarning = function() {};
18
+let lowPriorityWarning = lowPriorityWarningWithoutStack;
19
20
if (__DEV__) {
25
- const printWarning = function(format, ...args) {
26
- let argIndex = 0;
27
- const message = 'Warning: ' + format.replace(/%s/g, () => args[argIndex++]);
28
- if (typeof console !== 'undefined') {
29
- console.warn(message);
30
- }
31
- try {
32
- // --- Welcome to debugging React ---
33
- // This error was thrown as a convenience so that you can use this stack
34
- // to find the callsite that caused this warning to fire.
35
- throw new Error(message);
36
- } catch (x) {}
37
- };
38
-
21
lowPriorityWarning = function(condition, format, ...args) {
40
- if (format === undefined) {
41
- throw new Error(
42
- '`lowPriorityWarning(condition, format, ...args)` requires a warning ' +
43
- 'message argument',
44
- );
45
- }
46
- if (!condition) {
47
- printWarning(format, ...args);
22
+ if (condition) {
23
+ return;
24
}
25
+ const ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
26
+ const stack = ReactDebugCurrentFrame.getStackAddendum();
27
+ // eslint-disable-next-line react-internal/warning-and-invariant-args
28
+ lowPriorityWarningWithoutStack(false, format + '%s', ...args, stack);
29
};
30
}
31
packages/shared/lowPriorityWarningWithoutStack.js
new
+52
@@ -0,0 +1,52 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ */
7
+
8
+/**
9
+ * Forked from fbjs/warning:
10
+ * https://github.com/facebook/fbjs/blob/e66ba20ad5be433eb54423f2b097d829324d9de6/packages/fbjs/src/__forks__/warning.js
11
+ *
12
+ * Only change is we use console.warn instead of console.error,
13
+ * and do nothing when 'console' is not supported.
14
+ * This really simplifies the code.
15
+ * ---
16
+ * Similar to invariant but only logs a warning if the condition is not met.
17
+ * This can be used to log issues in development environments in critical
18
+ * paths. Removing the logging code for production environments will keep the
19
+ * same logic and follow the same code paths.
20
+ */
21
+
22
+let lowPriorityWarningWithoutStack = function() {};
23
+
24
+if (__DEV__) {
25
+ const printWarning = function(format, ...args) {
26
+ let argIndex = 0;
27
+ const message = 'Warning: ' + format.replace(/%s/g, () => args[argIndex++]);
28
+ if (typeof console !== 'undefined') {
29
+ console.warn(message);
30
+ }
31
+ try {
32
+ // --- Welcome to debugging React ---
33
+ // This error was thrown as a convenience so that you can use this stack
34
+ // to find the callsite that caused this warning to fire.
35
+ throw new Error(message);
36
+ } catch (x) {}
37
+ };
38
+
39
+ lowPriorityWarningWithoutStack = function(condition, format, ...args) {
40
+ if (format === undefined) {
41
+ throw new Error(
42
+ '`lowPriorityWarningWithoutStack(condition, format, ...args)` requires a warning ' +
43
+ 'message argument',
44
+ );
45
+ }
46
+ if (!condition) {
47
+ printWarning(format, ...args);
48
+ }
49
+ };
50
+}
51
+
52
+export default lowPriorityWarningWithoutStack;
scripts/jest/matchers/__tests__/toWarnDev-test.js
+206
@@ -210,3 +210,209 @@ describe('toWarnDev', () => {
210
});
211
}
212
});
213
+
214
+describe('toLowPriorityWarnDev', () => {
215
+ it('does not fail if a warning contains a stack', () => {
216
+ expect(() => {
217
+ if (__DEV__) {
218
+ console.warn('Hello\n in div');
219
+ }
220
+ }).toLowPriorityWarnDev('Hello');
221
+ });
222
+
223
+ it('does not fail if all warnings contain a stack', () => {
224
+ expect(() => {
225
+ if (__DEV__) {
226
+ console.warn('Hello\n in div');
227
+ console.warn('Good day\n in div');
228
+ console.warn('Bye\n in div');
229
+ }
230
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye']);
231
+ });
232
+
233
+ it('does not fail if warnings without stack explicitly opt out', () => {
234
+ expect(() => {
235
+ if (__DEV__) {
236
+ console.warn('Hello');
237
+ }
238
+ }).toLowPriorityWarnDev('Hello', {withoutStack: true});
239
+ expect(() => {
240
+ if (__DEV__) {
241
+ console.warn('Hello');
242
+ console.warn('Good day');
243
+ console.warn('Bye');
244
+ }
245
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye'], {withoutStack: true});
246
+ });
247
+
248
+ it('does not fail when expected stack-less warning number matches the actual one', () => {
249
+ expect(() => {
250
+ if (__DEV__) {
251
+ console.warn('Hello\n in div');
252
+ console.warn('Good day');
253
+ console.warn('Bye\n in div');
254
+ }
255
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye'], {withoutStack: 1});
256
+ });
257
+
258
+ if (__DEV__) {
259
+ // Helper methods avoids invalid toWarn().toThrow() nesting
260
+ // See no-to-warn-dev-within-to-throw
261
+ const expectToWarnAndToThrow = (expectBlock, expectedErrorMessage) => {
262
+ let caughtError;
263
+ try {
264
+ expectBlock();
265
+ } catch (error) {
266
+ caughtError = error;
267
+ }
268
+ expect(caughtError).toBeDefined();
269
+ expect(caughtError.message).toContain(expectedErrorMessage);
270
+ };
271
+
272
+ it('fails if a warning does not contain a stack', () => {
273
+ expectToWarnAndToThrow(() => {
274
+ expect(() => {
275
+ console.warn('Hello');
276
+ }).toLowPriorityWarnDev('Hello');
277
+ }, 'Received warning unexpectedly does not include a component stack');
278
+ });
279
+
280
+ it('fails if some warnings do not contain a stack', () => {
281
+ expectToWarnAndToThrow(() => {
282
+ expect(() => {
283
+ console.warn('Hello\n in div');
284
+ console.warn('Good day\n in div');
285
+ console.warn('Bye');
286
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye']);
287
+ }, 'Received warning unexpectedly does not include a component stack');
288
+ expectToWarnAndToThrow(() => {
289
+ expect(() => {
290
+ console.warn('Hello');
291
+ console.warn('Good day\n in div');
292
+ console.warn('Bye\n in div');
293
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye']);
294
+ }, 'Received warning unexpectedly does not include a component stack');
295
+ expectToWarnAndToThrow(() => {
296
+ expect(() => {
297
+ console.warn('Hello\n in div');
298
+ console.warn('Good day');
299
+ console.warn('Bye\n in div');
300
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye']);
301
+ }, 'Received warning unexpectedly does not include a component stack');
302
+ expectToWarnAndToThrow(() => {
303
+ expect(() => {
304
+ console.warn('Hello');
305
+ console.warn('Good day');
306
+ console.warn('Bye');
307
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye']);
308
+ }, 'Received warning unexpectedly does not include a component stack');
309
+ });
310
+
311
+ it('fails if warning is expected to not have a stack, but does', () => {
312
+ expectToWarnAndToThrow(() => {
313
+ expect(() => {
314
+ console.warn('Hello\n in div');
315
+ }).toLowPriorityWarnDev('Hello', {withoutStack: true});
316
+ }, 'Received warning unexpectedly includes a component stack');
317
+ expectToWarnAndToThrow(() => {
318
+ expect(() => {
319
+ console.warn('Hello\n in div');
320
+ console.warn('Good day');
321
+ console.warn('Bye\n in div');
322
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye'], {
323
+ withoutStack: true,
324
+ });
325
+ }, 'Received warning unexpectedly includes a component stack');
326
+ });
327
+
328
+ it('fails if expected stack-less warning number does not match the actual one', () => {
329
+ expectToWarnAndToThrow(() => {
330
+ expect(() => {
331
+ console.warn('Hello\n in div');
332
+ console.warn('Good day');
333
+ console.warn('Bye\n in div');
334
+ }).toLowPriorityWarnDev(['Hello', 'Good day', 'Bye'], {
335
+ withoutStack: 4,
336
+ });
337
+ }, 'Expected 4 warnings without a component stack but received 1');
338
+ });
339
+
340
+ it('fails if withoutStack is invalid', () => {
341
+ expectToWarnAndToThrow(() => {
342
+ expect(() => {
343
+ console.warn('Hi');
344
+ }).toLowPriorityWarnDev('Hi', {withoutStack: null});
345
+ }, 'Instead received object');
346
+ expectToWarnAndToThrow(() => {
347
+ expect(() => {
348
+ console.warn('Hi');
349
+ }).toLowPriorityWarnDev('Hi', {withoutStack: {}});
350
+ }, 'Instead received object');
351
+ expectToWarnAndToThrow(() => {
352
+ expect(() => {
353
+ console.warn('Hi');
354
+ }).toLowPriorityWarnDev('Hi', {withoutStack: 'haha'});
355
+ }, 'Instead received string');
356
+ });
357
+
358
+ it('fails if the argument number does not match', () => {
359
+ expectToWarnAndToThrow(() => {
360
+ expect(() => {
361
+ console.warn('Hi %s', 'Sara', 'extra');
362
+ }).toLowPriorityWarnDev('Hi', {withoutStack: true});
363
+ }, 'Received 2 arguments for a message with 1 placeholders');
364
+
365
+ expectToWarnAndToThrow(() => {
366
+ expect(() => {
367
+ console.warn('Hi %s');
368
+ }).toLowPriorityWarnDev('Hi', {withoutStack: true});
369
+ }, 'Received 0 arguments for a message with 1 placeholders');
370
+ });
371
+
372
+ it('fails if stack is passed twice', () => {
373
+ expectToWarnAndToThrow(() => {
374
+ expect(() => {
375
+ console.warn('Hi %s%s', '\n in div', '\n in div');
376
+ }).toLowPriorityWarnDev('Hi');
377
+ }, 'Received more than one component stack for a warning');
378
+ });
379
+
380
+ it('fails if multiple strings are passed without an array wrapper', () => {
381
+ expectToWarnAndToThrow(() => {
382
+ expect(() => {
383
+ console.warn('Hi \n in div');
384
+ }).toLowPriorityWarnDev('Hi', 'Bye');
385
+ }, 'toWarnDev() second argument, when present, should be an object');
386
+ expectToWarnAndToThrow(() => {
387
+ expect(() => {
388
+ console.warn('Hi \n in div');
389
+ console.warn('Bye \n in div');
390
+ }).toLowPriorityWarnDev('Hi', 'Bye');
391
+ }, 'toWarnDev() second argument, when present, should be an object');
392
+ expectToWarnAndToThrow(() => {
393
+ expect(() => {
394
+ console.warn('Hi \n in div');
395
+ console.warn('Wow \n in div');
396
+ console.warn('Bye \n in div');
397
+ }).toLowPriorityWarnDev('Hi', 'Bye');
398
+ }, 'toWarnDev() second argument, when present, should be an object');
399
+ expectToWarnAndToThrow(() => {
400
+ expect(() => {
401
+ console.warn('Hi \n in div');
402
+ console.warn('Wow \n in div');
403
+ console.warn('Bye \n in div');
404
+ }).toLowPriorityWarnDev('Hi', 'Wow', 'Bye');
405
+ }, 'toWarnDev() second argument, when present, should be an object');
406
+ });
407
+
408
+ it('fails on more than two arguments', () => {
409
+ expectToWarnAndToThrow(() => {
410
+ expect(() => {
411
+ console.warn('Hi \n in div');
412
+ console.warn('Wow \n in div');
413
+ console.warn('Bye \n in div');
414
+ }).toLowPriorityWarnDev('Hi', undefined, 'Bye');
415
+ }, 'toWarnDev() received more than two arguments.');
416
+ });
417
+ }
418
+});
scripts/print-warnings/print-warnings.js
+3
-1
@@ -53,7 +53,8 @@ function transform(file, enc, cb) {
53
if (
54
callee.isIdentifier({name: 'warning'}) ||
55
callee.isIdentifier({name: 'warningWithoutStack'}) ||
56
- callee.isIdentifier({name: 'lowPriorityWarning'})
56
+ callee.isIdentifier({name: 'lowPriorityWarning'}) ||
57
+ callee.isIdentifier({name: 'lowPriorityWarningWithoutStack'})
58
) {
59
const node = astPath.node;
60
@@ -82,6 +83,7 @@ function transform(file, enc, cb) {
83
gs([
84
'packages/**/*.js',
85
'!packages/shared/warning.js',
86
+ '!packages/shared/lowPriorityWarning.js',
87
'!packages/react-devtools*/**/*.js',
88
'!**/__tests__/**/*.js',
89
'!**/__mocks__/**/*.js',
scripts/rollup/forks.js
+2
-2
@@ -181,12 +181,12 @@ const forks = Object.freeze({
181
},
182
183
// This logic is forked on www to ignore some warnings.
184
- 'shared/lowPriorityWarning': (bundleType, entry) => {
184
+ 'shared/lowPriorityWarningWithoutStack': (bundleType, entry) => {
185
switch (bundleType) {
186
case FB_WWW_DEV:
187
case FB_WWW_PROD:
188
case FB_WWW_PROFILING:
189
- return 'shared/forks/lowPriorityWarning.www.js';
189
+ return 'shared/forks/lowPriorityWarningWithoutStack.www.js';
190
default:
191
return null;
192
}