Fix Suspense-wrapping heuristic

Author: Brian Vaughn

packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap

@@ -753,6 +753,19 @@ Array [
 ]
 `;
 
+exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 10 1`] = `
+Array [
+  1,
+  2,
+]
+`;
+
+exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 13 1`] = `
+Array [
+  2,
+]
+`;
+
 exports[`ProfilingCache should collect data for each rendered fiber: imported data 1`] = `
 Object {
   "dataForRoots": Array [

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

@@ -716,4 +716,93 @@ describe('ProfilingCache', () => {
       TestRenderer.create(<Validator commitIndex={0} rootID={rootID} />);
     });
   });
+
+  // See https://github.com/facebook/react/issues/18831
+  it('should not crash during route transitions with Suspense', () => {
+    const RouterContext = React.createContext();
+
+    function App() {
+      return (
+        <Router>
+          <Switch>
+            <Route path="/">
+              <Home />
+            </Route>
+            <Route path="/about">
+              <About />
+            </Route>
+          </Switch>
+        </Router>
+      );
+    }
+
+    const Home = () => {
+      return (
+        <React.Suspense>
+          <Link path="/about">Home</Link>
+        </React.Suspense>
+      );
+    };
+
+    const About = () => <div>About</div>;
+
+    // Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Router.js
+    function Router({children}) {
+      const [path, setPath] = React.useState('/');
+      return (
+        <RouterContext.Provider value={{path, setPath}}>
+          {children}
+        </RouterContext.Provider>
+      );
+    }
+
+    // Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Switch.js
+    function Switch({children}) {
+      return (
+        <RouterContext.Consumer>
+          {context => {
+            let element = null;
+            React.Children.forEach(children, child => {
+              if (context.path === child.props.path) {
+                element = child.props.children;
+              }
+            });
+            return element ? React.cloneElement(element) : null;
+          }}
+        </RouterContext.Consumer>
+      );
+    }
+
+    // Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Route.js
+    function Route({children, path}) {
+      return null;
+    }
+
+    const linkRef = React.createRef();
+
+    // Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/Link.js
+    function Link({children, path}) {
+      return (
+        <RouterContext.Consumer>
+          {context => {
+            return (
+              <button ref={linkRef} onClick={() => context.setPath(path)}>
+                {children}
+              </button>
+            );
+          }}
+        </RouterContext.Consumer>
+      );
+    }
+
+    const {Simulate} = require('react-dom/test-utils');
+
+    const container = document.createElement('div');
+    utils.act(() => ReactDOM.render(<App />, container));
+    expect(container.textContent).toBe('Home');
+    utils.act(() => store.profilerStore.startProfiling());
+    utils.act(() => Simulate.click(linkRef.current));
+    utils.act(() => store.profilerStore.stopProfiling());
+    expect(container.textContent).toBe('About');
+  });
 });

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

@@ -7,7 +7,7 @@
  * @flow
  */
 
-import {gte} from 'semver';
+import {gt, gte} from 'semver';
 import {
   ComponentFilterDisplayName,
   ComponentFilterElementType,
@@ -153,7 +153,8 @@ export function getInternalReactConstants(
   // **********************************************************
   // The section below is copied from files in React repo.
   // Keep it in sync, and add version guards if it changes.
-  if (gte(version, '16.6.0-beta.0')) {
+  if (gt(version, '16.13.1')) {
+    // TODO (Offscreen) Update the version number above to reflect the first Offscreen alpha/beta release.
     ReactTypeOfWork = {
       Block: 22,
       ClassComponent: 1,
@@ -181,6 +182,34 @@ export function getInternalReactConstants(
       SuspenseListComponent: 19, // Experimental
       YieldComponent: -1, // Removed
     };
+  } else if (gte(version, '16.6.0-beta.0')) {
+    ReactTypeOfWork = {
+      Block: 22,
+      ClassComponent: 1,
+      ContextConsumer: 9,
+      ContextProvider: 10,
+      CoroutineComponent: -1, // Removed
+      CoroutineHandlerPhase: -1, // Removed
+      DehydratedSuspenseComponent: 18, // Behind a flag
+      ForwardRef: 11,
+      Fragment: 7,
+      FunctionComponent: 0,
+      HostComponent: 5,
+      HostPortal: 4,
+      HostRoot: 3,
+      HostText: 6,
+      IncompleteClassComponent: 17,
+      IndeterminateComponent: 2,
+      LazyComponent: 16,
+      MemoComponent: 14,
+      Mode: 8,
+      OffscreenComponent: -1, // Experimental
+      Profiler: 12,
+      SimpleMemoComponent: 15,
+      SuspenseComponent: 13,
+      SuspenseListComponent: 19, // Experimental
+      YieldComponent: -1, // Removed
+    };
   } else if (gte(version, '16.4.3-alpha')) {
     ReactTypeOfWork = {
       Block: -1, // Doesn't exist yet
@@ -452,14 +481,16 @@ export function attach(
   const debug = (name: string, fiber: Fiber, parentFiber: ?Fiber): void => {
     if (__DEBUG__) {
       const displayName = getDisplayNameForFiber(fiber) || 'null';
+      const id = getFiberID(fiber);
       const parentDisplayName =
         (parentFiber != null && getDisplayNameForFiber(parentFiber)) || 'null';
+      const parentID = parentFiber ? getFiberID(parentFiber) : '';
       // NOTE: calling getFiberID or getPrimaryFiber is unsafe here
       // because it will put them in the map. For now, we'll omit them.
       // TODO: better debugging story for this.
       console.log(
-        `[renderer] %c${name} %c${displayName} %c${
-          parentFiber ? parentDisplayName : ''
+        `[renderer] %c${name} %c${displayName} (${id}) %c${
+          parentFiber ? `${parentDisplayName} (${parentID})` : ''
         }`,
         'color: red; font-weight: bold;',
         'color: blue;',
@@ -1076,6 +1107,10 @@ export function attach(
   }
 
   function recordMount(fiber: Fiber, parentFiber: Fiber | null) {
+    if (__DEBUG__) {
+      debug('recordMount()', fiber, parentFiber);
+    }
+
     const isRoot = fiber.tag === HostRoot;
     const id = getFiberID(getPrimaryFiber(fiber));
 
@@ -1130,6 +1165,10 @@ export function attach(
   }
 
   function recordUnmount(fiber: Fiber, isSimulated: boolean) {
+    if (__DEBUG__) {
+      debug('recordUnmount()', fiber);
+    }
+
     if (trackedPathMatchFiber !== null) {
       // We're in the process of trying to restore previous selection.
       // If this fiber matched but is being unmounted, there's no use trying.
@@ -1215,7 +1254,8 @@ export function attach(
       // because we don't want to highlight every host node inside of a newly mounted subtree.
     }
 
-    if (fiber.tag === ReactTypeOfWork.SuspenseComponent) {
+    const isSuspense = fiber.tag === ReactTypeOfWork.SuspenseComponent;
+    if (isSuspense) {
       const isTimedOut = fiber.memoizedState !== null;
       if (isTimedOut) {
         // Special case: if Suspense mounts in a timed-out state,