DevTools: Add break-on-warn feature (#19048)

This commit adds a new tab to the Settings modal: Debugging

This new tab has the append component stacks feature and a new one: break on warn

This new feature adds a debugger statement into the console override

packages/react-devtools-shared/src/__tests__/console-test.js

@@ -44,7 +44,10 @@ describe('console', () => {
 
     // Note the Console module only patches once,
     // so it's important to patch the test console before injection.
-    patchConsole();
+    patchConsole({
+      appendComponentStack: true,
+      breakOnWarn: false,
+    });
 
     const inject = global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject;
     global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => {
@@ -79,7 +82,10 @@ describe('console', () => {
   it('should only patch the console once', () => {
     const {error, warn} = fakeConsole;
 
-    patchConsole();
+    patchConsole({
+      appendComponentStack: true,
+      breakOnWarn: false,
+    });
 
     expect(fakeConsole.error).toBe(error);
     expect(fakeConsole.warn).toBe(warn);
@@ -330,7 +336,10 @@ describe('console', () => {
     expect(mockError.mock.calls[0]).toHaveLength(1);
     expect(mockError.mock.calls[0][0]).toBe('error');
 
-    patchConsole();
+    patchConsole({
+      appendComponentStack: true,
+      breakOnWarn: false,
+    });
     act(() => ReactDOM.render(<Child />, document.createElement('div')));
 
     expect(mockWarn).toHaveBeenCalledTimes(2);

packages/react-devtools-shared/src/backend/agent.js

@@ -161,8 +161,8 @@ export default class Agent extends EventEmitter<{|
     );
     bridge.addListener('shutdown', this.shutdown);
     bridge.addListener(
-      'updateAppendComponentStack',
-      this.updateAppendComponentStack,
+      'updateConsolePatchSettings',
+      this.updateConsolePatchSettings,
     );
     bridge.addListener('updateComponentFilters', this.updateComponentFilters);
     bridge.addListener('viewAttributeSource', this.viewAttributeSource);
@@ -443,13 +443,19 @@ export default class Agent extends EventEmitter<{|
     }
   };
 
-  updateAppendComponentStack = (appendComponentStack: boolean) => {
+  updateConsolePatchSettings = ({
+    appendComponentStack,
+    breakOnConsoleErrors,
+  }: {|
+    appendComponentStack: boolean,
+    breakOnConsoleErrors: boolean,
+  |}) => {
     // If the frontend preference has change,
     // or in the case of React Native- if the backend is just finding out the preference-
     // then install or uninstall the console overrides.
     // It's safe to call these methods multiple times, so we don't need to worry about that.
-    if (appendComponentStack) {
-      patchConsole();
+    if (appendComponentStack || breakOnConsoleErrors) {
+      patchConsole({appendComponentStack, breakOnConsoleErrors});
     } else {
       unpatchConsole();
     }

packages/react-devtools-shared/src/backend/console.js

@@ -80,9 +80,25 @@ export function registerRenderer(renderer: ReactRenderer): void {
   }
 }
 
+const consoleSettingsRef = {
+  appendComponentStack: false,
+  breakOnConsoleErrors: false,
+};
+
 // Patches whitelisted console methods to append component stack for the current fiber.
 // Call unpatch() to remove the injected behavior.
-export function patch(): void {
+export function patch({
+  appendComponentStack,
+  breakOnConsoleErrors,
+}: {
+  appendComponentStack: boolean,
+  breakOnConsoleErrors: boolean,
+}): void {
+  // Settings may change after we've patched the console.
+  // Using a shared ref allows the patch function to read the latest values.
+  consoleSettingsRef.appendComponentStack = appendComponentStack;
+  consoleSettingsRef.breakOnConsoleErrors = breakOnConsoleErrors;
+
   if (unpatchFn !== null) {
     // Don't patch twice.
     return;
@@ -105,40 +121,56 @@ export function patch(): void {
         targetConsole[method]);
 
       const overrideMethod = (...args) => {
-        try {
-          // If we are ever called with a string that already has a component stack, e.g. a React error/warning,
-          // don't append a second stack.
-          const lastArg = args.length > 0 ? args[args.length - 1] : null;
-          const alreadyHasComponentStack =
-            lastArg !== null &&
-            (PREFIX_REGEX.test(lastArg) ||
-              ROW_COLUMN_NUMBER_REGEX.test(lastArg));
+        const latestAppendComponentStack =
+          consoleSettingsRef.appendComponentStack;
+        const latestBreakOnConsoleErrors =
+          consoleSettingsRef.breakOnConsoleErrors;
 
-          if (!alreadyHasComponentStack) {
-            // If there's a component stack for at least one of the injected renderers, append it.
-            // We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?)
-            // eslint-disable-next-line no-for-of-loops/no-for-of-loops
-            for (const {
-              currentDispatcherRef,
-              getCurrentFiber,
-              workTagMap,
-            } of injectedRenderers.values()) {
-              const current: ?Fiber = getCurrentFiber();
-              if (current != null) {
-                const componentStack = getStackByFiberInDevAndProd(
-                  workTagMap,
-                  current,
-                  currentDispatcherRef,
-                );
-                if (componentStack !== '') {
-                  args.push(componentStack);
+        if (latestAppendComponentStack) {
+          try {
+            // If we are ever called with a string that already has a component stack, e.g. a React error/warning,
+            // don't append a second stack.
+            const lastArg = args.length > 0 ? args[args.length - 1] : null;
+            const alreadyHasComponentStack =
+              lastArg !== null &&
+              (PREFIX_REGEX.test(lastArg) ||
+                ROW_COLUMN_NUMBER_REGEX.test(lastArg));
+
+            if (!alreadyHasComponentStack) {
+              // If there's a component stack for at least one of the injected renderers, append it.
+              // We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?)
+              // eslint-disable-next-line no-for-of-loops/no-for-of-loops
+              for (const {
+                currentDispatcherRef,
+                getCurrentFiber,
+                workTagMap,
+              } of injectedRenderers.values()) {
+                const current: ?Fiber = getCurrentFiber();
+                if (current != null) {
+                  const componentStack = getStackByFiberInDevAndProd(
+                    workTagMap,
+                    current,
+                    currentDispatcherRef,
+                  );
+                  if (componentStack !== '') {
+                    args.push(componentStack);
+                  }
+                  break;
                 }
-                break;
               }
             }
+          } catch (error) {
+            // Don't let a DevTools or React internal error interfere with logging.
           }
-        } catch (error) {
-          // Don't let a DevTools or React internal error interfere with logging.
+        }
+
+        if (latestBreakOnConsoleErrors) {
+          // --- Welcome to debugging with React DevTools ---
+          // This debugger statement means that you've enabled the "break on warnings" feature.
+          // Use the browser's Call Stack panel to step out of this override function-
+          // to where the original warning or error was logged.
+          // eslint-disable-next-line no-debugger
+          debugger;
         }
 
         originalMethod(...args);

packages/react-devtools-shared/src/backend/renderer.js

@@ -430,11 +430,18 @@ export function attach(
   if (process.env.NODE_ENV !== 'test') {
     registerRendererWithConsole(renderer);
 
-    // The renderer interface can't read this preference directly,
+    // The renderer interface can't read these preferences directly,
     // because it is stored in localStorage within the context of the extension.
     // It relies on the extension to pass the preference through via the global.
-    if (window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ !== false) {
-      patchConsole();
+    const appendComponentStack =
+      window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ !== false;
+    const breakOnConsoleErrors =
+      window.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ === true;
+    if (appendComponentStack || breakOnConsoleErrors) {
+      patchConsole({
+        appendComponentStack,
+        breakOnConsoleErrors,
+      });
     }
   }
 

packages/react-devtools-shared/src/bridge.js

@@ -85,6 +85,11 @@ type NativeStyleEditor_SetValueParams = {|
   value: string,
 |};
 
+type UpdateConsolePatchSettingsParams = {|
+  appendComponentStack: boolean,
+  breakOnConsoleErrors: boolean,
+|};
+
 type BackendEvents = {|
   extensionBackendInitialized: [],
   inspectedElement: [InspectedElementPayload],
@@ -133,8 +138,8 @@ type FrontendEvents = {|
   stopInspectingNative: [boolean],
   stopProfiling: [],
   storeAsGlobal: [StoreAsGlobalParams],
-  updateAppendComponentStack: [boolean],
   updateComponentFilters: [Array<ComponentFilter>],
+  updateConsolePatchSettings: [UpdateConsolePatchSettingsParams],
   viewAttributeSource: [ViewAttributeSourceParams],
   viewElementSource: [ElementAndRendererID],
 

packages/react-devtools-shared/src/constants.js

@@ -27,6 +27,9 @@ export const SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY =
 export const SESSION_STORAGE_RELOAD_AND_PROFILE_KEY =
   'React::DevTools::reloadAndProfile';
 
+export const LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS =
+  'React::DevTools::breakOnConsoleErrors';
+
 export const LOCAL_STORAGE_SHOULD_PATCH_CONSOLE_KEY =
   'React::DevTools::appendComponentStack';
 

packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js

@@ -0,0 +1,53 @@
+/**
+ * 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.
+ *
+ * @flow
+ */
+
+import * as React from 'react';
+import {useContext} from 'react';
+import {SettingsContext} from './SettingsContext';
+
+import styles from './SettingsShared.css';
+
+export default function DebuggingSettings(_: {||}) {
+  const {
+    appendComponentStack,
+    breakOnConsoleErrors,
+    setAppendComponentStack,
+    setBreakOnConsoleErrors,
+  } = useContext(SettingsContext);
+
+  return (
+    <div className={styles.Settings}>
+      <div className={styles.Setting}>
+        <label>
+          <input
+            type="checkbox"
+            checked={appendComponentStack}
+            onChange={({currentTarget}) =>
+              setAppendComponentStack(currentTarget.checked)
+            }
+          />{' '}
+          Append component stacks to console warnings and errors.
+        </label>
+      </div>
+
+      <div className={styles.Setting}>
+        <label>
+          <input
+            type="checkbox"
+            checked={breakOnConsoleErrors}
+            onChange={({currentTarget}) =>
+              setBreakOnConsoleErrors(currentTarget.checked)
+            }
+          />{' '}
+          Break on warnings
+        </label>
+      </div>
+    </div>
+  );
+}

packages/react-devtools-shared/src/devtools/views/Settings/GeneralSettings.js

@@ -17,9 +17,7 @@ import styles from './SettingsShared.css';
 
 export default function GeneralSettings(_: {||}) {
   const {
-    appendComponentStack,
     displayDensity,
-    setAppendComponentStack,
     setDisplayDensity,
     setTheme,
     setTraceUpdatesEnabled,
@@ -71,19 +69,6 @@ export default function GeneralSettings(_: {||}) {
         </div>
       )}
 
-      <div className={styles.Setting}>
-        <label>
-          <input
-            type="checkbox"
-            checked={appendComponentStack}
-            onChange={({currentTarget}) =>
-              setAppendComponentStack(currentTarget.checked)
-            }
-          />{' '}
-          Append component stacks to console warnings and errors.
-        </label>
-      </div>
-
       <div className={styles.ReleaseNotes}>
         <a
           className={styles.ReleaseNotesLink}

packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js

@@ -18,6 +18,7 @@ import {
 import {
   COMFORTABLE_LINE_HEIGHT,
   COMPACT_LINE_HEIGHT,
+  LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS,
   LOCAL_STORAGE_SHOULD_PATCH_CONSOLE_KEY,
   LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY,
 } from 'react-devtools-shared/src/constants';
@@ -40,6 +41,9 @@ type Context = {|
   appendComponentStack: boolean,
   setAppendComponentStack: (value: boolean) => void,
 
+  breakOnConsoleErrors: boolean,
+  setBreakOnConsoleErrors: (value: boolean) => void,
+
   theme: Theme,
   setTheme(value: Theme): void,
 
@@ -79,6 +83,13 @@ function SettingsContextController({
     appendComponentStack,
     setAppendComponentStack,
   ] = useLocalStorage<boolean>(LOCAL_STORAGE_SHOULD_PATCH_CONSOLE_KEY, true);
+  const [
+    breakOnConsoleErrors,
+    setBreakOnConsoleErrors,
+  ] = useLocalStorage<boolean>(
+    LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS,
+    false,
+  );
   const [
     traceUpdatesEnabled,
     setTraceUpdatesEnabled,
@@ -133,8 +144,11 @@ function SettingsContextController({
   }, [browserTheme, theme, documentElements]);
 
   useEffect(() => {
-    bridge.send('updateAppendComponentStack', appendComponentStack);
-  }, [bridge, appendComponentStack]);
+    bridge.send('updateConsolePatchSettings', {
+      appendComponentStack,
+      breakOnConsoleErrors,
+    });
+  }, [bridge, appendComponentStack, breakOnConsoleErrors]);
 
   useEffect(() => {
     bridge.send('setTraceUpdatesEnabled', traceUpdatesEnabled);
@@ -143,12 +157,14 @@ function SettingsContextController({
   const value = useMemo(
     () => ({
       appendComponentStack,
+      breakOnConsoleErrors,
       displayDensity,
       lineHeight:
         displayDensity === 'compact'
           ? COMPACT_LINE_HEIGHT
           : COMFORTABLE_LINE_HEIGHT,
       setAppendComponentStack,
+      setBreakOnConsoleErrors,
       setDisplayDensity,
       setTheme,
       setTraceUpdatesEnabled,
@@ -157,8 +173,10 @@ function SettingsContextController({
     }),
     [
       appendComponentStack,
+      breakOnConsoleErrors,
       displayDensity,
       setAppendComponentStack,
+      setBreakOnConsoleErrors,
       setDisplayDensity,
       setTheme,
       setTraceUpdatesEnabled,

packages/react-devtools-shared/src/devtools/views/Settings/SettingsModal.js

@@ -20,6 +20,7 @@ import {
   useSubscription,
 } from '../hooks';
 import ComponentsSettings from './ComponentsSettings';
+import DebuggingSettings from './DebuggingSettings';
 import GeneralSettings from './GeneralSettings';
 import ProfilerSettings from './ProfilerSettings';
 
@@ -78,15 +79,18 @@ function SettingsModalImpl(_: {||}) {
 
   let view = null;
   switch (selectedTabID) {
+    case 'components':
+      view = <ComponentsSettings />;
+      break;
+    case 'debugging':
+      view = <DebuggingSettings />;
+      break;
     case 'general':
       view = <GeneralSettings />;
       break;
     case 'profiler':
       view = <ProfilerSettings />;
       break;
-    case 'components':
-      view = <ComponentsSettings />;
-      break;
     default:
       break;
   }
@@ -119,6 +123,11 @@ const tabs = [
     icon: 'settings',
     label: 'General',
   },
+  {
+    id: 'debugging',
+    icon: 'bug',
+    label: 'Debugging',
+  },
   {
     id: 'components',
     icon: 'components',

packages/react-devtools-shared/src/hook.js

@@ -174,6 +174,11 @@ export function installHook(target: any): DevToolsHook | null {
     // Don't patch in test environments because we don't want to interfere with Jest's own console overrides.
     if (process.env.NODE_ENV !== 'test') {
       try {
+        const appendComponentStack =
+          window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ !== false;
+        const breakOnConsoleErrors =
+          window.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ === true;
+
         // The installHook() function is injected by being stringified in the browser,
         // so imports outside of this function do not get included.
         //
@@ -181,9 +186,12 @@ export function installHook(target: any): DevToolsHook | null {
         // but Webpack wraps imports with an object (e.g. _backend_console__WEBPACK_IMPORTED_MODULE_0__)
         // and the object itself will be undefined as well for the reasons mentioned above,
         // so we use try/catch instead.
-        if (window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ !== false) {
+        if (appendComponentStack || breakOnConsoleErrors) {
           registerRendererWithConsole(renderer);
-          patchConsole();
+          patchConsole({
+            appendComponentStack,
+            breakOnConsoleErrors,
+          });
         }
       } catch (error) {}
     }

packages/react-devtools-shared/src/utils.js

@@ -31,6 +31,7 @@ import {
 import {ElementTypeRoot} from 'react-devtools-shared/src/types';
 import {
   LOCAL_STORAGE_FILTER_PREFERENCES_KEY,
+  LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS,
   LOCAL_STORAGE_SHOULD_PATCH_CONSOLE_KEY,
 } from './constants';
 import {ComponentFilterElementType, ElementTypeHostComponent} from './types';
@@ -248,6 +249,25 @@ export function setAppendComponentStack(value: boolean): void {
   );
 }
 
+export function getBreakOnConsoleErrors(): boolean {
+  try {
+    const raw = localStorageGetItem(
+      LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS,
+    );
+    if (raw != null) {
+      return JSON.parse(raw);
+    }
+  } catch (error) {}
+  return true;
+}
+
+export function setBreakOnConsoleErrors(value: boolean): void {
+  localStorageSetItem(
+    LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS,
+    JSON.stringify(value),
+  );
+}
+
 export function separateDisplayNameAndHOCs(
   displayName: string | null,
   type: ElementType,