Replace console.warn/error with a custom wrapper at build time

facebook · Dec 13, 2019 · 06478a6 · 06478a6
1 parent 782c7b6
commit 06478a6
Show file tree

Hide file tree

Showing 34 changed files with 407 additions and 382 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -33,7 +33,8 @@ module.exports = {
     'comma-dangle': [ERROR, 'always-multiline'],
     'consistent-return': OFF,
     'dot-location': [ERROR, 'property'],
-    'dot-notation': ERROR,
+    // We use console['error']() as a signal to not transform it:
+    'dot-notation': [ERROR, {allowPattern: '^(error|warn)$'}],
     'eol-last': ERROR,
     eqeqeq: [ERROR, 'allow-null'],
     indent: OFF,
@@ -135,6 +136,18 @@ module.exports = {
         'jest/valid-expect-in-promise': ERROR,
       },
     },
+    {
+      files: [
+        '**/__tests__/**/*.js',
+        'scripts/**/*.js',
+        'packages/*/npm/**/*.js',
+        'packages/react-devtools*/**/*.js'
+      ],
+      rules: {
+        'react-internal/no-production-logging': OFF,
+        'react-internal/warning-args': OFF,
+      },
+    },
     {
       files: ['packages/react-native-renderer/**/*.js'],
       globals: {

diff --git a/packages/legacy-events/ResponderEventPlugin.js b/packages/legacy-events/ResponderEventPlugin.js
@@ -515,9 +515,11 @@ const ResponderEventPlugin = {
       if (trackedTouchCount >= 0) {
         trackedTouchCount -= 1;
       } else {
-        console.warn(
-          'Ended a touch event which was not counted in `trackedTouchCount`.',
-        );
+        if (__DEV__) {
+          console.warn(
+            'Ended a touch event which was not counted in `trackedTouchCount`.',
+          );
+        }
         return null;
       }
     }

diff --git a/packages/legacy-events/ResponderTouchHistoryStore.js b/packages/legacy-events/ResponderTouchHistoryStore.js
@@ -129,12 +129,15 @@ function recordTouchMove(touch: Touch): void {
     touchRecord.currentTimeStamp = timestampForTouch(touch);
     touchHistory.mostRecentTimeStamp = timestampForTouch(touch);
   } else {
-    console.warn(
-      'Cannot record touch move without a touch start.\n' + 'Touch Move: %s\n',
-      'Touch Bank: %s',
-      printTouch(touch),
-      printTouchBank(),
-    );
+    if (__DEV__) {
+      console.warn(
+        'Cannot record touch move without a touch start.\n' +
+          'Touch Move: %s\n' +
+          'Touch Bank: %s',
+        printTouch(touch),
+        printTouchBank(),
+      );
+    }
   }
 }
 
@@ -150,12 +153,15 @@ function recordTouchEnd(touch: Touch): void {
     touchRecord.currentTimeStamp = timestampForTouch(touch);
     touchHistory.mostRecentTimeStamp = timestampForTouch(touch);
   } else {
-    console.warn(
-      'Cannot record touch end without a touch start.\n' + 'Touch End: %s\n',
-      'Touch Bank: %s',
-      printTouch(touch),
-      printTouchBank(),
-    );
+    if (__DEV__) {
+      console.warn(
+        'Cannot record touch end without a touch start.\n' +
+          'Touch End: %s\n' +
+          'Touch Bank: %s',
+        printTouch(touch),
+        printTouchBank(),
+      );
+    }
   }
 }
 

diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js
@@ -211,6 +211,7 @@ if (__DEV__) {
       const protocol = window.location.protocol;
       // Don't warn in exotic cases like chrome-extension://.
       if (/^(https?|file):$/.test(protocol)) {
+        // eslint-disable-next-line react-internal/no-production-logging
         console.info(
           '%cDownload the React DevTools ' +
             'for a better development experience: ' +

diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
@@ -79,6 +79,7 @@ function act(callback: () => Thenable) {
   if (!__DEV__) {
     if (didWarnAboutUsingActInProd === false) {
       didWarnAboutUsingActInProd = true;
+      // eslint-disable-next-line react-internal/no-production-logging
       console.error(
         'act(...) is not supported in production builds of React, and might not behave as expected.',
       );

diff --git a/packages/react-interactions/events/src/dom/testing-library/domEnvironment.js b/packages/react-interactions/events/src/dom/testing-library/domEnvironment.js
@@ -22,7 +22,9 @@ export function hasPointerEvent() {
 export function setPointerEvent(bool) {
   const pointerCaptureFn = name => id => {
     if (typeof id !== 'number') {
-      console.error(`A pointerId must be passed to "${name}"`);
+      if (__DEV__) {
+        console.error('A pointerId must be passed to "%s"', name);
+      }
     }
   };
   global.PointerEvent = bool ? emptyFunction : undefined;

diff --git a/packages/react-is/src/ReactIs.js b/packages/react-is/src/ReactIs.js
@@ -25,7 +25,6 @@ import {
   REACT_SUSPENSE_TYPE,
 } from 'shared/ReactSymbols';
 import isValidElementType from 'shared/isValidElementType';
-import lowPriorityWarningWithoutStack from 'shared/lowPriorityWarningWithoutStack';
 
 export function typeOf(object: any) {
   if (typeof object === 'object' && object !== null) {
@@ -88,7 +87,7 @@ export function isAsyncMode(object: any) {
   if (__DEV__) {
     if (!hasWarnedAboutDeprecatedIsAsyncMode) {
       hasWarnedAboutDeprecatedIsAsyncMode = true;
-      lowPriorityWarningWithoutStack(
+      console.warn(
         'The ReactIs.isAsyncMode() alias has been deprecated, ' +
           'and will be removed in React 17+. Update your code to use ' +
           'ReactIs.isConcurrentMode() instead. It has the exact same API.',

diff --git a/packages/react-native-renderer/src/NativeMethodsMixinUtils.js b/packages/react-native-renderer/src/NativeMethodsMixinUtils.js
@@ -66,17 +66,19 @@ export function throwOnStylesProp(component: any, props: any) {
 }
 
 export function warnForStyleProps(props: any, validAttributes: any) {
-  for (const key in validAttributes.style) {
-    if (!(validAttributes[key] || props[key] === undefined)) {
-      console.error(
-        'You are setting the style `{ ' +
-          key +
-          ': ... }` as a prop. You ' +
-          'should nest it in a style object. ' +
-          'E.g. `{ style: { ' +
-          key +
-          ': ... } }`',
-      );
+  if (__DEV__) {
+    for (const key in validAttributes.style) {
+      if (!(validAttributes[key] || props[key] === undefined)) {
+        console.error(
+          'You are setting the style `{ %s' +
+            ': ... }` as a prop. You ' +
+            'should nest it in a style object. ' +
+            'E.g. `{ style: { %s' +
+            ': ... } }`',
+          key,
+          key,
+        );
+      }
     }
   }
 }
diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -638,6 +638,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
     if (!__DEV__) {
       if (didWarnAboutUsingActInProd === false) {
         didWarnAboutUsingActInProd = true;
+        // eslint-disable-next-line react-internal/no-production-logging
         console.error(
           'act(...) is not supported in production builds of React, and might not behave as expected.',
         );
@@ -1106,6 +1107,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       const root = roots.get(rootID);
       const rootContainer = rootContainers.get(rootID);
       if (!root || !rootContainer) {
+        // eslint-disable-next-line react-internal/no-production-logging
         console.log('Nothing rendered yet.');
         return;
       }
@@ -1208,6 +1210,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       log('FIBERS:');
       logFiber(root.current, 0);
 
+      // eslint-disable-next-line react-internal/no-production-logging
       console.log(...bufferedLog);
     },
 

diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js
@@ -20,9 +20,6 @@ export function logCapturedError(capturedError: CapturedError): void {
     return;
   }
 
-  // Prevent it from being seen by Babel transform.
-  const rawConsoleError = console.error;
-
   const error = (capturedError.error: any);
   if (__DEV__) {
     const {
@@ -47,7 +44,7 @@ export function logCapturedError(capturedError: CapturedError): void {
       // been accidental, we'll surface it anyway.
       // However, the browser would have silenced the original error
       // so we'll print it first, and then print the stack addendum.
-      rawConsoleError(error);
+      console['error'](error); // Don't transform to our wrapper
       // For a more detailed description of this block, see:
       // https://github.com/facebook/react/pull/13384
     }
@@ -81,11 +78,12 @@ export function logCapturedError(capturedError: CapturedError): void {
     // We don't include the original error message and JS stack because the browser
     // has already printed it. Even if the application swallows the error, it is still
     // displayed by the browser thanks to the DEV-only fake event trick in ReactErrorUtils.
-    rawConsoleError(combinedMessage);
+    console['error'](combinedMessage); // Don't transform to our wrapper
   } else {
     // In production, we print the error directly.
     // This will include the message, the JS stack, and anything the browser wants to show.
     // We pass the error object instead of custom message so that the browser displays the error natively.
-    rawConsoleError(error);
+    // eslint-disable-next-line react-internal/no-production-logging
+    console['error'](error); // Don't transform to our wrapper
   }
 }
diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js
@@ -60,6 +60,7 @@ function act(callback: () => Thenable) {
   if (!__DEV__) {
     if (didWarnAboutUsingActInProd === false) {
       didWarnAboutUsingActInProd = true;
+      // eslint-disable-next-line react-internal/no-production-logging
       console.error(
         'act(...) is not supported in production builds of React, and might not behave as expected.',
       );

diff --git a/packages/react/src/__tests__/ReactClassEquivalence-test.js b/packages/react/src/__tests__/ReactClassEquivalence-test.js
@@ -28,7 +28,8 @@ describe('ReactClassEquivalence', () => {
 function runJest(testFile) {
   const cwd = process.cwd();
   const extension = process.platform === 'win32' ? '.cmd' : '';
-  const result = spawnSync('yarn' + extension, ['test', testFile], {
+  const command = __DEV__ ? 'test' : 'test-prod';
+  const result = spawnSync('yarn' + extension, [command, testFile], {
     cwd,
     env: Object.assign({}, process.env, {
       REACT_CLASS_EQUIVALENCE_TEST: 'true',

diff --git a/packages/scheduler/src/SchedulerProfiling.js b/packages/scheduler/src/SchedulerProfiling.js
@@ -69,7 +69,8 @@ function logEvent(entries) {
     if (eventLogIndex + 1 > eventLogSize) {
       eventLogSize *= 2;
       if (eventLogSize > MAX_EVENT_LOG_SIZE) {
-        console.error(
+        // Using console['error'] to evade Babel and ESLint
+        console['error'](
           "Scheduler Profiling: Event log exceeded maximum size. Don't " +
             'forget to call `stopLoggingProfilingEvents()`.',
         );

diff --git a/packages/scheduler/src/__tests__/SchedulerProfiling-test.js b/packages/scheduler/src/__tests__/SchedulerProfiling-test.js
@@ -544,7 +544,7 @@ Task 1 [Normal]              │                    █████████
     }
 
     expect(console.error).toHaveBeenCalledTimes(1);
-    expect(console.error.calls.argsFor(0)[0]).toContain(
+    expect(console.error.calls.argsFor(0)[0]).toBe(
       "Scheduler Profiling: Event log exceeded maximum size. Don't forget " +
         'to call `stopLoggingProfilingEvents()`.',
     );

diff --git a/packages/scheduler/src/forks/SchedulerHostConfig.default.js b/packages/scheduler/src/forks/SchedulerHostConfig.default.js
@@ -81,14 +81,16 @@ if (
     const cancelAnimationFrame = window.cancelAnimationFrame;
     // TODO: Remove fb.me link
     if (typeof requestAnimationFrame !== 'function') {
-      console.error(
+      // Using console['error'] to evade Babel and ESLint
+      console['error'](
         "This browser doesn't support requestAnimationFrame. " +
           'Make sure that you load a ' +
           'polyfill in older browsers. https://fb.me/react-polyfills',
       );
     }
     if (typeof cancelAnimationFrame !== 'function') {
-      console.error(
+      // Using console['error'] to evade Babel and ESLint
+      console['error'](
         "This browser doesn't support cancelAnimationFrame. " +
           'Make sure that you load a ' +
           'polyfill in older browsers. https://fb.me/react-polyfills',
@@ -169,7 +171,8 @@ if (
 
   forceFrameRate = function(fps) {
     if (fps < 0 || fps > 125) {
-      console.error(
+      // Using console['error'] to evade Babel and ESLint
+      console['error'](
         'forceFrameRate takes a positive int between 0 and 125, ' +
           'forcing framerates higher than 125 fps is not unsupported',
       );

diff --git a/packages/shared/consoleWithStackDev.js b/packages/shared/consoleWithStackDev.js
@@ -0,0 +1,63 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import ReactSharedInternals from 'shared/ReactSharedInternals';
+
+// In DEV, calls to console.warn and console.error get replaced
+// by calls to these methods by a Babel plugin.
+//
+// In PROD (or in packages without access to React internals),
+// they are left as they are instead.
+
+export function warn(format, ...args) {
+  if (__DEV__) {
+    printWarning('warn', format, args);
+  }
+}
+
+export function error(format, ...args) {
+  if (__DEV__) {
+    printWarning('error', format, args);
+  }
+}
+
+function printWarning(level, format, args) {
+  if (__DEV__) {
+    const hasExistingStack =
+      args.length > 0 &&
+      typeof args[args.length - 1] === 'string' &&
+      args[args.length - 1].indexOf('\n    in') === 0;
+
+    if (!hasExistingStack) {
+      const ReactDebugCurrentFrame =
+        ReactSharedInternals.ReactDebugCurrentFrame;
+      const stack = ReactDebugCurrentFrame.getStackAddendum();
+      if (stack !== '') {
+        format += '%s';
+        args = args.concat([stack]);
+      }
+    }
+
+    const argsWithFormat = args.map(item => '' + item);
+    // Careful: RN currently depends on this prefix
+    argsWithFormat.unshift('Warning: ' + format);
+    // We intentionally don't use spread (or .apply) directly because it
+    // breaks IE9: https://github.com/facebook/react/issues/13610
+    // eslint-disable-next-line react-internal/no-production-logging
+    Function.prototype.apply.call(console[level], console, argsWithFormat);
+
+    try {
+      // --- Welcome to debugging React ---
+      // This error was thrown as a convenience so that you can use this stack
+      // to find the callsite that caused this warning to fire.
+      let argIndex = 0;
+      const message =
+        'Warning: ' + format.replace(/%s/g, () => args[argIndex++]);
+      throw new Error(message);
+    } catch (x) {}
+  }
+}
diff --git a/packages/shared/enqueueTask.js b/packages/shared/enqueueTask.js
@@ -7,8 +7,6 @@
  * @flow
  */
 
-import warningWithoutStack from 'shared/warningWithoutStack';
-
 let didWarnAboutMessageChannel = false;
 let enqueueTask;
 try {
@@ -28,7 +26,7 @@ try {
       if (didWarnAboutMessageChannel === false) {
         didWarnAboutMessageChannel = true;
         if (typeof MessageChannel === 'undefined') {
-          warningWithoutStack(
+          console.error(
             'This browser does not have a MessageChannel implementation, ' +
               'so enqueuing tasks via await act(async () => ...) will fail. ' +
               'Please file an issue at https://github.com/facebook/react/issues ' +

diff --git a/packages/shared/forks/consoleWithStackDev.www.js b/packages/shared/forks/consoleWithStackDev.www.js
@@ -0,0 +1,22 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// This refers to a WWW module.
+const warningWWW = require('warning');
+
+export function warn() {
+  // TODO: use different level for "warn".
+  const args = Array.prototype.slice.call(arguments);
+  args.unshift(false);
+  warningWWW.apply(null, args);
+}
+
+export function error() {
+  const args = Array.prototype.slice.call(arguments);
+  args.unshift(false);
+  warningWWW.apply(null, args);
+}