DevTools always overrides the dispatcher when shallow rendering (#20011)

This is done so that any effects scheduled by the shallow render are thrown away.

Unlike the code this was forked from (in ReactComponentStackFrame) DevTools should override the dispatcher even when DevTools is compiled in production mode, because the app itself may be in development mode and log errors/warnings.

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

@@ -0,0 +1,105 @@
+/**
+ * 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
+ */
+
+function normalizeCodeLocInfo(str) {
+  if (typeof str !== 'string') {
+    return str;
+  }
+  // This special case exists only for the special source location in
+  // ReactElementValidator. That will go away if we remove source locations.
+  str = str.replace(/Check your code at .+?:\d+/g, 'Check your code at **');
+  // V8 format:
+  //  at Component (/path/filename.js:123:45)
+  // React format:
+  //    in Component (at filename.js:123)
+  return str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
+    return '\n    in ' + name + ' (at **)';
+  });
+}
+
+describe('component stack', () => {
+  let React;
+  let ReactDOM;
+  let act;
+  let mockError;
+  let mockWarn;
+
+  beforeEach(() => {
+    // Intercept native console methods before DevTools bootstraps.
+    // Normalize component stack locations.
+    mockError = jest.fn();
+    mockWarn = jest.fn();
+    console.error = (...args) => {
+      mockError(...args.map(normalizeCodeLocInfo));
+    };
+    console.warn = (...args) => {
+      mockWarn(...args.map(normalizeCodeLocInfo));
+    };
+
+    const utils = require('./utils');
+    act = utils.act;
+
+    React = require('react');
+    ReactDOM = require('react-dom');
+  });
+
+  it('should log the current component stack along with an error or warning', () => {
+    const Grandparent = () => <Parent />;
+    const Parent = () => <Child />;
+    const Child = () => {
+      console.error('Test error.');
+      console.warn('Test warning.');
+      return null;
+    };
+
+    const container = document.createElement('div');
+
+    act(() => ReactDOM.render(<Grandparent />, container));
+
+    expect(mockError).toHaveBeenCalledWith(
+      'Test error.',
+      '\n    in Child (at **)' +
+        '\n    in Parent (at **)' +
+        '\n    in Grandparent (at **)',
+    );
+    expect(mockWarn).toHaveBeenCalledWith(
+      'Test warning.',
+      '\n    in Child (at **)' +
+        '\n    in Parent (at **)' +
+        '\n    in Grandparent (at **)',
+    );
+  });
+
+  // This test should have caught #19911
+  // but didn't because both DevTools and ReactDOM are running in the same memory space,
+  // so the case we're testing against (DevTools prod build and React DEV build) doesn't exist.
+  // It would be nice to figure out a way to test this combination at some point...
+  xit('should disable the current dispatcher before shallow rendering so no effects get scheduled', () => {
+    let useEffectCount = 0;
+
+    const Example = props => {
+      React.useEffect(() => {
+        useEffectCount++;
+        expect(props).toBeDefined();
+      }, [props]);
+      console.warn('Warning to trigger appended component stacks.');
+      return null;
+    };
+
+    const container = document.createElement('div');
+    act(() => ReactDOM.render(<Example test="abc" />, container));
+
+    expect(useEffectCount).toBe(1);
+
+    expect(mockWarn).toHaveBeenCalledWith(
+      'Warning to trigger appended component stacks.',
+      '\n    in Example (at **)',
+    );
+  });
+});

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

@@ -85,14 +85,16 @@ export function describeNativeComponentFrame(
   Error.prepareStackTrace = undefined;
 
   reentry = true;
-  let previousDispatcher;
-  if (__DEV__) {
-    previousDispatcher = currentDispatcherRef.current;
-    // Set the dispatcher in DEV because this might be call in the render function
-    // for warnings.
-    currentDispatcherRef.current = null;
-    disableLogs();
-  }
+
+  // Override the dispatcher so effects scheduled by this shallow render are thrown away.
+  //
+  // Note that unlike the code this was forked from (in ReactComponentStackFrame)
+  // DevTools should override the dispatcher even when DevTools is compiled in production mode,
+  // because the app itself may be in development mode and log errors/warnings.
+  const previousDispatcher = currentDispatcherRef.current;
+  currentDispatcherRef.current = null;
+  disableLogs();
+
   try {
     // This should throw.
     if (construct) {
@@ -188,10 +190,8 @@ export function describeNativeComponentFrame(
 
     Error.prepareStackTrace = previousPrepareStackTrace;
 
-    if (__DEV__) {
-      currentDispatcherRef.current = previousDispatcher;
-      reenableLogs();
-    }
+    currentDispatcherRef.current = previousDispatcher;
+    reenableLogs();
   }
   // Fallback to just using the name if we couldn't make it throw.
   const name = fn ? fn.displayName || fn.name : '';