Interaction tracking ref-counting bug fixes (WIP) (facebook#13590)

* Added new (failing) suspense+interaction tests * Add new tracing+suspense test harness fixture * Refactored interaction tracing to fix ref counting bug
jetoneza · Jan 23, 2019 · 95dad07 · 95dad07
1 parent 9321426
commit 95dad07
Show file tree

Hide file tree

Showing 7 changed files with 925 additions and 225 deletions.
diff --git a/fixtures/tracing/test.html b/fixtures/tracing/test.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html style="width: 100%; height: 100%;">
+  <head>
+    <meta charset="utf-8">
+    <title>Test tracing UMD</title>
+    <style>
+      body {
+        font-family: sans-serif;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="root"></div>
+    <!-- Load the tracing API before react to test that it's lazily evaluated -->
+    <script src="../../build/node_modules/scheduler/umd/scheduler.development.js"></script>
+    <script src="../../build/node_modules/scheduler/umd/scheduler-tracing.development.js"></script>
+    <script src="../../build/node_modules/react/umd/react.development.js"></script>
+    <script src="../../build/node_modules/react-dom/umd/react-dom.development.js"></script>
+    <script src="./test.js"></script>
+  </body>
+</html>
diff --git a/fixtures/tracing/test.js b/fixtures/tracing/test.js
@@ -0,0 +1,103 @@
+const {createElement, Component, Placeholder} = React;
+const {unstable_createRoot: createRoot} = ReactDOM;
+const {
+  unstable_subscribe: subscribe,
+  unstable_trace: trace,
+  unstable_wrap: wrap,
+} = SchedulerTracing;
+
+const createLogger = (backgroundColor, color, enabled) => (
+  message,
+  ...args
+) => {
+  if (enabled === false) return;
+  console.groupCollapsed(
+    `%c${message}`,
+    `background-color: ${backgroundColor}; color: ${color}; padding: 2px 4px;`,
+    ...args
+  );
+  console.log(
+    new Error('stack').stack
+      .split('\n')
+      .slice(2)
+      .join('\n')
+  );
+  console.groupEnd();
+};
+
+window.log = {
+  app: createLogger('#37474f', '#fff'),
+  interaction: createLogger('#6a1b9a', '#fff'),
+  react: createLogger('#ff5722', '#fff'),
+  tracing: createLogger('#2962ff', '#fff'),
+  work: createLogger('#e1bee7', '#000'),
+};
+
+// Fake suspense
+const resolvedValues = {};
+const read = key => {
+  if (!resolvedValues[key]) {
+    log.app(`Suspending for "${key}" ...`);
+    throw new Promise(
+      wrap(resolve => {
+        setTimeout(
+          wrap(() => {
+            log.app(`Loaded "${key}" ...`);
+            resolvedValues[key] = true;
+            resolve(key);
+          }),
+          1000
+        );
+      })
+    );
+  }
+  return key;
+};
+
+const TestApp = () =>
+  createElement(
+    Placeholder,
+    {delayMs: 100, fallback: createElement(PlaceholderText)},
+    createElement(SuspendingChild, {text: 'foo'}),
+    createElement(SuspendingChild, {text: 'bar'}),
+    createElement(SuspendingChild, {text: 'baz'})
+  );
+
+const PlaceholderText = () => 'Loading ...';
+
+const SuspendingChild = ({text}) => {
+  const resolvedValue = read(text);
+  return resolvedValue;
+};
+
+subscribe({
+  onInteractionScheduledWorkCompleted: interaction =>
+    log.interaction(
+      'onInteractionScheduledWorkCompleted',
+      JSON.stringify(interaction)
+    ),
+  onInteractionTraced: interaction =>
+    log.interaction('onInteractionTraced', JSON.stringify(interaction)),
+  onWorkCanceled: interactions =>
+    log.work('onWorkCanceled', JSON.stringify(Array.from(interactions))),
+  onWorkScheduled: interactions =>
+    log.work('onWorkScheduled', JSON.stringify(Array.from(interactions))),
+  onWorkStarted: interactions =>
+    log.work('onWorkStarted', JSON.stringify(Array.from(interactions))),
+  onWorkStopped: interactions =>
+    log.work('onWorkStopped', JSON.stringify(Array.from(interactions))),
+});
+
+const element = document.getElementById('root');
+trace('initial_render', performance.now(), () => {
+  const root = createRoot(element);
+  const batch = root.createBatch();
+  log.app('batch.render()');
+  batch.render(createElement(TestApp));
+  batch.then(
+    wrap(() => {
+      log.app('batch.commit()');
+      batch.commit();
+    })
+  );
+});
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -41,13 +41,15 @@ import {
   Update,
   Ref,
 } from 'shared/ReactSideEffectTags';
+import {captureWillSyncRenderPlaceholder} from './ReactFiberScheduler';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import {
   enableGetDerivedStateFromCatch,
   enableSuspense,
   debugRenderPhaseSideEffects,
   debugRenderPhaseSideEffectsForStrictMode,
   enableProfilerTimer,
+  enableSchedulerTracing,
 } from 'shared/ReactFeatureFlags';
 import invariant from 'shared/invariant';
 import getComponentName from 'shared/getComponentName';
@@ -825,6 +827,13 @@ function updatePlaceholderComponent(
 
     let nextDidTimeout;
     if (current !== null && workInProgress.updateQueue !== null) {
+      if (enableSchedulerTracing) {
+        // Handle special case of rendering a Placeholder for a sync, suspended tree.
+        // We flag this to properly trace and count interactions.
+        // Otherwise interaction pending count will be decremented too many times.
+        captureWillSyncRenderPlaceholder();
+      }
+
       // We're outside strict mode. Something inside this Placeholder boundary
       // suspended during the last commit. Switch to the placholder.
       workInProgress.updateQueue = null;