[DO NOT MERGE] Prototype showing component errors/warnings within the Components tree

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

@@ -37,7 +37,7 @@ import type {
   RendererID,
   RendererInterface,
 } from './types';
-import type {ComponentFilter} from '../types';
+import type {ComponentFilter, ErrorOrWarning} from '../types';
 
 const debug = (methodName, ...args) => {
   if (__DEBUG__) {
@@ -439,6 +439,10 @@ export default class Agent extends EventEmitter<{|
     }
   };
 
+  onErrorsAndWarnings = (errorsAndWarnings: Array<ErrorOrWarning>) => {
+    this._bridge.send('errorsAndWarnings', errorsAndWarnings);
+  };
+
   onTraceUpdates = (nodes: Set<NativeType>) => {
     this.emit('traceUpdates', nodes);
   };

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

@@ -17,11 +17,18 @@ const APPEND_STACK_TO_METHODS = ['error', 'trace', 'warn'];
 
 const FRAME_REGEX = /\n {4}in /;
 
+type OnErrorOrWarning = (
+  fiber: Fiber,
+  type: 'error' | 'warn',
+  args: Array<any>,
+) => void;
+
 const injectedRenderers: Map<
   ReactRenderer,
   {|
     getCurrentFiber: () => Fiber | null,
     getDisplayNameForFiber: (fiber: Fiber) => string | null,
+    onErrorOrWarning: ?OnErrorOrWarning,
   |},
 > = new Map();
 
@@ -48,7 +55,10 @@ export function dangerous_setTargetConsoleForTesting(
 // v16 renderers should use this method to inject internals necessary to generate a component stack.
 // These internals will be used if the console is patched.
 // Injecting them separately allows the console to easily be patched or un-patched later (at runtime).
-export function registerRenderer(renderer: ReactRenderer): void {
+export function registerRenderer(
+  renderer: ReactRenderer,
+  onErrorOrWarning?: OnErrorOrWarning,
+): void {
   const {getCurrentFiber, findFiberByHostInstance, version} = renderer;
 
   // Ignore React v15 and older because they don't expose a component stack anyway.
@@ -62,6 +72,7 @@ export function registerRenderer(renderer: ReactRenderer): void {
     injectedRenderers.set(renderer, {
       getCurrentFiber,
       getDisplayNameForFiber,
+      onErrorOrWarning,
     });
   }
 }
@@ -104,22 +115,37 @@ export function patch(): void {
             for (let {
               getCurrentFiber,
               getDisplayNameForFiber,
+              onErrorOrWarning,
             } of injectedRenderers.values()) {
               let current: ?Fiber = getCurrentFiber();
               let ownerStack: string = '';
-              while (current != null) {
-                const name = getDisplayNameForFiber(current);
-                const owner = current._debugOwner;
-                const ownerName =
-                  owner != null ? getDisplayNameForFiber(owner) : null;
-
-                ownerStack += describeComponentFrame(
-                  name,
-                  current._debugSource,
-                  ownerName,
-                );
-
-                current = owner;
+              if (current != null) {
+                if (method === 'error' || method === 'warn') {
+                  if (typeof onErrorOrWarning === 'function') {
+                    // TODO (inline errors) The renderer is injected in two places:
+                    // 1. First by "react-devtools-shared/src/hook" which isn't stateful and doesn't supply onErrorOrWarning()
+                    // 2. Second by "react-devtools-shared/src/backend/renderer" which is and does
+                    //
+                    // We should probably move the queueing+batching mechanism from backend/renderer to this file,
+                    // so that it handles warnings logged during initial mount (potentially before step 2 above).
+                    onErrorOrWarning(current, method, args);
+                  }
+                }
+
+                while (current != null) {
+                  const name = getDisplayNameForFiber(current);
+                  const owner = current._debugOwner;
+                  const ownerName =
+                    owner != null ? getDisplayNameForFiber(owner) : null;
+
+                  ownerStack += describeComponentFrame(
+                    name,
+                    current._debugSource,
+                    ownerName,
+                  );
+
+                  current = owner;
+                }
               }
 
               if (ownerStack !== '') {
@@ -129,13 +155,15 @@ export function patch(): void {
             }
           }
         } catch (error) {
+          console.log(error);
           // Don't let a DevTools or React internal error interfere with logging.
         }
 
         originalMethod(...args);
       };
 
       overrideMethod.__REACT_DEVTOOLS_ORIGINAL_METHOD__ = originalMethod;
+      originalMethod.__REACT_DEVTOOLS_OVERRIDE_METHOD__ = overrideMethod;
 
       // $FlowFixMe property error|warn is not writable.
       targetConsole[method] = overrideMethod;

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

@@ -43,6 +43,7 @@ export function initBackend(
       agent.onUnsupportedRenderer(id);
     }),
 
+    hook.sub('errorsAndWarnings', agent.onErrorsAndWarnings),
     hook.sub('operations', agent.onHookOperations),
     hook.sub('traceUpdates', agent.onTraceUpdates),
 

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

@@ -493,13 +493,69 @@ export function attach(
     typeof setSuspenseHandler === 'function' &&
     typeof scheduleUpdate === 'function';
 
+  type PendingErrorOrWarning = {|
+    args: Array<any>,
+    fiber: Fiber,
+    type: 'error' | 'warn',
+  |};
+
+  let flushErrorOrWarningUpdatesTimoutID: TimeoutID | null = null;
+  const pendingErrorOrWarnings: Array<PendingErrorOrWarning> = [];
+
+  function onErrorOrWarning(
+    fiber: Fiber,
+    type: 'error' | 'warn',
+    args: Array<any>,
+  ): void {
+    // There are a few places that errors might be logged:
+    // 1. During render (either for initial mount or an update)
+    // 2. During commit effects (both active and passive)
+    // 3. During unmounts (or commit effect cleanup functions)
+    //
+    // Initial render presents a special challenge-
+    // since neither backend nor frontend Store know about a Fiber until it has mounted (and committed).
+    // In this case, we need to hold onto errors until the subsequent commit hook is called.
+    //
+    // Passive effects also present a special challenge-
+    // since the commit hook is called before passive effects, are run,
+    // meaning that we might not want to wait until the subsequent commit hook to notify the frontend.
+    //
+    // For this reason, warnings/errors are sent to the frontend periodically (on a timer).
+    // When processing errors, any Fiber that is not still registered is assumed to be unmounted.
+    pendingErrorOrWarnings.push({fiber, type, args});
+
+    if (flushErrorOrWarningUpdatesTimoutID === null) {
+      flushErrorOrWarningUpdatesTimoutID = setTimeout(
+        flushErrorOrWarningUpdates,
+        1000,
+      );
+    }
+  }
+
+  function flushErrorOrWarningUpdates() {
+    flushErrorOrWarningUpdatesTimoutID = null;
+
+    hook.emit(
+      'errorsAndWarnings',
+      pendingErrorOrWarnings
+        .filter(({fiber}) => isFiberMounted(fiber))
+        .map(({args, fiber, type}) => ({
+          id: getFiberID(getPrimaryFiber(fiber)),
+          type,
+          // TODO (inline errors) Send JSON-serialized args
+        })),
+    );
+
+    pendingErrorOrWarnings.splice(0);
+  }
+
   // Patching the console enables DevTools to do a few useful things:
   // * Append component stacks to warnings and error messages
   // * Disable logging during re-renders to inspect hooks (see inspectHooksOfFiber)
   //
   // 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') {
-    registerRendererWithConsole(renderer);
+    registerRendererWithConsole(renderer, onErrorOrWarning);
 
     // The renderer interface can't read this preference directly,
     // because it is stored in localStorage within the context of the extension.
@@ -757,6 +813,17 @@ export function attach(
     return fiber;
   }
 
+  function isFiberMounted(fiber: Fiber): boolean {
+    if (primaryFibers.has(fiber)) {
+      return true;
+    }
+    const {alternate} = fiber;
+    if (alternate != null && primaryFibers.has(alternate)) {
+      return true;
+    }
+    return false;
+  }
+
   const fiberToIDMap: Map<Fiber, number> = new Map();
   const idToFiberMap: Map<number, Fiber> = new Map();
   const primaryFibers: Set<Fiber> = new Set();

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

@@ -9,7 +9,7 @@
 
 import EventEmitter from 'events';
 
-import type {ComponentFilter, Wall} from './types';
+import type {ComponentFilter, ErrorOrWarning, Wall} from './types';
 import type {
   InspectedElementPayload,
   OwnersList,
@@ -70,6 +70,7 @@ type NativeStyleEditor_SetValueParams = {|
 |};
 
 type BackendEvents = {|
+  errorsAndWarnings: [Array<ErrorOrWarning>],
   extensionBackendInitialized: [],
   inspectedElement: [InspectedElementPayload],
   isBackendStorageAPISupported: [boolean],

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

@@ -29,7 +29,7 @@ import {printStore} from './utils';
 import ProfilerStore from './ProfilerStore';
 
 import type {Element} from './views/Components/types';
-import type {ComponentFilter, ElementType} from '../types';
+import type {ComponentFilter, ElementType, ErrorOrWarning} from '../types';
 import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
 
 const debug = (methodName, ...args) => {
@@ -174,6 +174,7 @@ export default class Store extends EventEmitter<{|
     }
 
     this._bridge = bridge;
+    bridge.addListener('errorsAndWarnings', this.onBridgeErrorsAndWarnings);
     bridge.addListener('operations', this.onBridgeOperations);
     bridge.addListener(
       'overrideComponentFilters',
@@ -704,6 +705,10 @@ export default class Store extends EventEmitter<{|
     this.emit('supportsNativeStyleEditor');
   };
 
+  onBridgeErrorsAndWarnings = (errorsAndWarnings: Array<ErrorOrWarning>) => {
+    console.log('onBridgeErrorsAndWarnings', errorsAndWarnings);
+  };
+
   onBridgeOperations = (operations: Array<number>) => {
     if (__DEBUG__) {
       console.groupCollapsed('onBridgeOperations');

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

@@ -75,3 +75,8 @@ export type ComponentFilter =
   | BooleanComponentFilter
   | ElementTypeComponentFilter
   | RegExpComponentFilter;
+
+export type ErrorOrWarning = {|
+  id: number,
+  type: 'error' | 'warn',
+|};

packages/react-devtools-shell/src/app/InlineWarnings/index.js

@@ -0,0 +1,51 @@
+/** @flow */
+
+import React, {Fragment, useEffect, useRef, useState} from 'react';
+
+function WarnDuringRender() {
+  console.warn('This warning fires during every render');
+  return null;
+}
+
+function WarnOnMount() {
+  useEffect(() => {
+    console.warn('This warning fires on initial mount only');
+  }, []);
+  return null;
+}
+
+function WarnOnUpdate() {
+  const didMountRef = useRef(false);
+  useEffect(() => {
+    if (didMountRef.current) {
+      console.warn('This warning fires on every update');
+    } else {
+      didMountRef.current = true;
+    }
+  });
+  return null;
+}
+
+function WarnOnUnmount() {
+  useEffect(() => {
+    return () => {
+      console.warn('This warning fires on unmount');
+    };
+  }, []);
+  return null;
+}
+
+export default function ElementTypes() {
+  const [count, setCount] = useState(0);
+  const handleClick = () => setCount(count + 1);
+  return (
+    <Fragment>
+      <h1>Inline warnings</h1>
+      <button onClick={handleClick}>Update {count > 0 ? count : ''}</button>
+      <WarnDuringRender />
+      <WarnOnMount />
+      <WarnOnUpdate />
+      {count === 0 ? <WarnOnUnmount /> : null}
+    </Fragment>
+  );
+}

packages/react-devtools-shell/src/app/console.js

@@ -11,7 +11,12 @@ function ignoreStrings(
   methodName: string,
   stringsToIgnore: Array<string>,
 ): void {
-  const originalMethod = console[methodName];
+  // HACKY In the test harness, DevTools overrides the parent window's console.
+  // Our test app code uses the iframe's console though.
+  // To simulate a more accurate end-ot-end ienvironment,
+  // the shell's console patching should pass through to the parent override methods.
+  const originalMethod = window.parent.console[methodName];
+
   console[methodName] = (...args) => {
     const maybeString = args[0];
     if (typeof maybeString === 'string') {

packages/react-devtools-shell/src/app/index.js

@@ -12,6 +12,7 @@ import Iframe from './Iframe';
 import EditableProps from './EditableProps';
 import ElementTypes from './ElementTypes';
 import Hydration from './Hydration';
+import InlineWarnings from './InlineWarnings';
 import InspectableElements from './InspectableElements';
 import InteractionTracing from './InteractionTracing';
 import PriorityLevels from './PriorityLevels';
@@ -31,7 +32,10 @@ ignoreErrors([
   'Warning: Unsafe lifecycle methods',
   'Warning: %s is deprecated in StrictMode.', // findDOMNode
 ]);
-ignoreWarnings(['Warning: componentWillReceiveProps is deprecated']);
+ignoreWarnings([
+  'Warning: componentWillReceiveProps is deprecated',
+  'Warning: componentWillReceiveProps has been renamed',
+]);
 
 const roots = [];
 
@@ -53,6 +57,7 @@ function mountTestApp() {
   mountHelper(Hydration);
   mountHelper(ElementTypes);
   mountHelper(EditableProps);
+  mountHelper(InlineWarnings);
   mountHelper(PriorityLevels);
   mountHelper(ReactNativeWeb);
   mountHelper(Toggle);