From ffbb86b361e5cfec0c3c5dcc2f5ab829e7f54527 Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Wed, 28 Dec 2016 20:41:40 -0800
Subject: [PATCH 1/6] Wrap top-level mount and unmount in syncUpdates

For legacy purposes. Only enabled in the DOM renderer. We can remove
this in a future release when we enable incremental-by-default.

This change is unobservable because syncUpdates actually schedules
Task updates when it is called from inside another batch. The correct
behavior is to recursively begin another batch of work. We will fix it
in a subsequent commit.
---
 src/renderers/dom/fiber/ReactDOMFiber.js | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js
index 20d851dcee68..a4ed6d3e547b 100644
--- a/src/renderers/dom/fiber/ReactDOMFiber.js
+++ b/src/renderers/dom/fiber/ReactDOMFiber.js
@@ -322,9 +322,15 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,
     while (container.lastChild) {
       container.removeChild(container.lastChild);
     }
-    root = container._reactRootContainer = DOMRenderer.createContainer(container);
+    const newRoot = DOMRenderer.createContainer(container);
+    root = container._reactRootContainer = newRoot;
+    // Initial mount is always sync, even if we're in a batch.
+    DOMRenderer.syncUpdates(() => {
+      DOMRenderer.updateContainer(children, newRoot, parentComponent, callback);
+    });
+  } else {
+    DOMRenderer.updateContainer(children, root, parentComponent, callback);
   }
-  DOMRenderer.updateContainer(children, root, parentComponent, callback);
   return DOMRenderer.getPublicRootInstance(root);
 }
 
@@ -346,8 +352,11 @@ var ReactDOM = {
   unmountComponentAtNode(container : DOMContainerElement) {
     warnAboutUnstableUse();
     if (container._reactRootContainer) {
-      return renderSubtreeIntoContainer(null, null, container, () => {
-        container._reactRootContainer = null;
+      // Unmount is always sync, even if we're in a batch.
+      return DOMRenderer.syncUpdates(() => {
+        return renderSubtreeIntoContainer(null, null, container, () => {
+          container._reactRootContainer = null;
+        });
       });
     }
   },

From 3926a3566fc8c28758b9f816006ea2495c63c71d Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Wed, 28 Dec 2016 20:51:42 -0800
Subject: [PATCH 2/6] If an error is thrown and there's no nextUnitOfWork, it's
 fatal error

This can only be caused by a bug in the renderer, but we should handle
it gracefully anyway.

Added a TODO to change capturedErrors and failedBoundaries so that they
are sets of instances (stateNodes) rather than fibers, to avoid having
to check the alternate. (This outside the scope of this PR.)
---
 .../shared/fiber/ReactFiberScheduler.js       | 68 ++++++++++---------
 1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
index 63cea47ee4c1..bc14bf369339 100644
--- a/src/renderers/shared/fiber/ReactFiberScheduler.js
+++ b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -705,39 +705,40 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
       } catch (error) {
         // We caught an error during either the begin or complete phases.
         const failedWork = nextUnitOfWork;
-        if (!failedWork) {
-          throw new Error('Should have nextUnitOfWork.');
-        }
-
-        // Reset the priority context to its value before reconcilation.
-        priorityContext = priorityContextBeforeReconciliation;
-
-        // "Capture" the error by finding the nearest boundary. If there is no
-        // error boundary, the nearest host container acts as one. If
-        // captureError returns null, the error was intentionally ignored.
-        const maybeBoundary = captureError(failedWork, error);
-        if (maybeBoundary) {
-          const boundary = maybeBoundary;
-
-          // Complete the boundary as if it rendered null. This will unmount
-          // the failed tree.
-          beginFailedWork(boundary.alternate, boundary, priorityLevel);
-
-          // The next unit of work is now the boundary that captured the error.
-          // Conceptually, we're unwinding the stack. We need to unwind the
-          // context stack, too, from the failed work to the boundary that
-          // captured the error.
-          // TODO: If we set the memoized props in beginWork instead of
-          // completeWork, rather than unwind the stack, we can just restart
-          // from the root. Can't do that until then because without memoized
-          // props, the nodes higher up in the tree will rerender unnecessarily.
-          if (failedWork) {
+        if (failedWork) {
+          // Reset the priority context to its value before reconcilation.
+          priorityContext = priorityContextBeforeReconciliation;
+
+          // "Capture" the error by finding the nearest boundary. If there is no
+          // error boundary, the nearest host container acts as one. If
+          // captureError returns null, the error was intentionally ignored.
+          const maybeBoundary = captureError(failedWork, error);
+          if (maybeBoundary) {
+            const boundary = maybeBoundary;
+
+            // Complete the boundary as if it rendered null. This will unmount
+            // the failed tree.
+            beginFailedWork(boundary.alternate, boundary, priorityLevel);
+
+            // The next unit of work is now the boundary that captured the error.
+            // Conceptually, we're unwinding the stack. We need to unwind the
+            // context stack, too, from the failed work to the boundary that
+            // captured the error.
+            // TODO: If we set the memoized props in beginWork instead of
+            // completeWork, rather than unwind the stack, we can just restart
+            // from the root. Can't do that until then because without memoized
+            // props, the nodes higher up in the tree will rerender unnecessarily.
             unwindContexts(failedWork, boundary);
+            nextUnitOfWork = completeUnitOfWork(boundary);
           }
-          nextUnitOfWork = completeUnitOfWork(boundary);
+          // Continue performing work
+          continue;
+        } else if (!fatalError) {
+          // There is no current unit of work. This is a worst-case scenario
+          // and should only be possible if there's a bug in the renderer, e.g.
+          // inside resetAfterCommit.
+          fatalError = error;
         }
-        // Continue performing work
-        continue;
       } finally {
         priorityContext = priorityContextBeforeReconciliation;
       }
@@ -900,6 +901,8 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   }
 
   function hasCapturedError(fiber : Fiber) : boolean {
+    // TODO: capturedErrors should store the boundary instance, to avoid needing
+    // to check the alternate.
     return Boolean(
       capturedErrors &&
       (capturedErrors.has(fiber) || (fiber.alternate && capturedErrors.has(fiber.alternate)))
@@ -907,11 +910,12 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   }
 
   function isFailedBoundary(fiber : Fiber) : boolean {
-    const res = Boolean(
+    // TODO: failedBoundaries should store the boundary instance, to avoid
+    // needing to check the alternate.
+    return Boolean(
       failedBoundaries &&
       (failedBoundaries.has(fiber) || (fiber.alternate && failedBoundaries.has(fiber.alternate)))
     );
-    return res;
   }
 
   function commitErrorHandling(effectfulFiber : Fiber) {

From 8090cf59e38653bf611d04f510b3ca32434e3d47 Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Wed, 28 Dec 2016 21:02:50 -0800
Subject: [PATCH 3/6] Allow performWork to be called recursively

Currently we assume that performWork is never called recursively.
Ideally that is the case. We shouldn't rely on recursion anywhere
in Fiber.

However, to support the use of syncUpdates as an opt-in to forced-
synchronous updates, we need the ability to call performWork
recursively.

At the beginning of performWork, the state of the scheduler is saved;
before exiting, that state is restored, so that the scheduler can
continue where it left off. I've decided to use local variables to stash
the previous state. We could also store them in a record and push it to
a stack, but this approach has the advantage of being isolated to
performWork.
---
 .../shared/fiber/ReactFiberScheduler.js       | 57 ++++++++++++-------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
index bc14bf369339..b4a8aeb7e615 100644
--- a/src/renderers/shared/fiber/ReactFiberScheduler.js
+++ b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -672,10 +672,29 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   }
 
   function performWork(priorityLevel : PriorityLevel, deadline : Deadline | null) {
-    if (isPerformingWork) {
-      throw new Error('performWork was called recursively.');
-    }
+    // If performWork is called recursively, we need to save the previous state
+    // of the scheduler so it can be restored before the function exits.
+    // Recursion is only possible when using syncUpdates.
+    const previousPriorityContext = priorityContext;
+    const previousPriorityContextBeforeReconciliation = priorityContextBeforeReconciliation;
+    const previousIsPerformingWork = isPerformingWork;
+    const previousNextEffect = nextEffect;
+    const previousCommitPhaseBoundaries = commitPhaseBoundaries;
+    const previousFirstUncaughtError = firstUncaughtError;
+    const previousFatalError = fatalError;
+    const previousIsCommitting = isCommitting;
+    const previousIsUnmounting = isUnmounting;
+
+    priorityContext = NoWork;
+    priorityContextBeforeReconciliation = NoWork;
     isPerformingWork = true;
+    nextEffect = null;
+    commitPhaseBoundaries = null;
+    firstUncaughtError = null;
+    fatalError = null;
+    isCommitting = false;
+    isUnmounting = false;
+
     const isPerformingDeferredWork = Boolean(deadline);
     let deadlineHasExpired = false;
 
@@ -779,22 +798,22 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
       }
     }
 
-    // We're done performing work. Time to clean up.
-    isPerformingWork = false;
-    capturedErrors = null;
-    failedBoundaries = null;
-
-    // It's now safe to throw errors.
-    if (fatalError) {
-      let e = fatalError;
-      fatalError = null;
-      firstUncaughtError = null;
-      throw e;
-    }
-    if (firstUncaughtError) {
-      let e = firstUncaughtError;
-      firstUncaughtError = null;
-      throw e;
+    const errorToThrow = fatalError || firstUncaughtError;
+
+    // We're done performing work. Restore the previous state of the scheduler.
+    priorityContext = previousPriorityContext;
+    priorityContextBeforeReconciliation = previousPriorityContextBeforeReconciliation;
+    isPerformingWork = previousIsPerformingWork;
+    nextEffect = previousNextEffect;
+    commitPhaseBoundaries = previousCommitPhaseBoundaries;
+    firstUncaughtError = previousFirstUncaughtError;
+    fatalError = previousFatalError;
+    isCommitting = previousIsCommitting;
+    isUnmounting = previousIsUnmounting;
+
+    // It's safe to throw any unhandled errors.
+    if (errorToThrow) {
+      throw errorToThrow;
     }
   }
 

From 21f6d4145fdaace4a8e856729b3566e0aaad62d1 Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Wed, 28 Dec 2016 21:12:34 -0800
Subject: [PATCH 4/6] syncUpdates prevents Synchronous priority from being
 downgraded to Task

Typically, a sync update is downgraded to Task priority if it's
scheduled within another batch of work, e.g. within a lifecycle like
componentDidMount. But syncUpdates should force sync updates to not
be downgraded to Task. This will cause performWork to be
called recursively.
---
 .../shared/fiber/ReactFiberScheduler.js       | 34 +++++++++++--------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
index b4a8aeb7e615..45fc8dbf36ae 100644
--- a/src/renderers/shared/fiber/ReactFiberScheduler.js
+++ b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -107,10 +107,12 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   // is thrown during reconciliation.
   let priorityContextBeforeReconciliation : PriorityLevel = NoWork;
 
-  // Keeps track of whether we're currently in a work loop. Used to batch
-  // nested updates.
+  // Keeps track of whether we're currently in a work loop.
   let isPerformingWork : boolean = false;
 
+  // Keeps track of whether sync updates should be downgraded to task updates.
+  let shouldDeferSyncUpdates : boolean = false;
+
   // The next work in progress fiber that we're currently working on.
   let nextUnitOfWork : ?Fiber = null;
   let nextPriorityLevel : PriorityLevel = NoWork;
@@ -678,6 +680,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
     const previousPriorityContext = priorityContext;
     const previousPriorityContextBeforeReconciliation = priorityContextBeforeReconciliation;
     const previousIsPerformingWork = isPerformingWork;
+    const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
     const previousNextEffect = nextEffect;
     const previousCommitPhaseBoundaries = commitPhaseBoundaries;
     const previousFirstUncaughtError = firstUncaughtError;
@@ -688,6 +691,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
     priorityContext = NoWork;
     priorityContextBeforeReconciliation = NoWork;
     isPerformingWork = true;
+    shouldDeferSyncUpdates = true;
     nextEffect = null;
     commitPhaseBoundaries = null;
     firstUncaughtError = null;
@@ -804,6 +808,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
     priorityContext = previousPriorityContext;
     priorityContextBeforeReconciliation = previousPriorityContextBeforeReconciliation;
     isPerformingWork = previousIsPerformingWork;
+    shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
     nextEffect = previousNextEffect;
     commitPhaseBoundaries = previousCommitPhaseBoundaries;
     firstUncaughtError = previousFirstUncaughtError;
@@ -1058,15 +1063,11 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
           // schedule a callback to perform work later.
           switch (priorityLevel) {
             case SynchronousPriority:
-              // Perform work immediately
               performWork(SynchronousPriority);
               return;
             case TaskPriority:
-              // If we're already performing work, Task work will be flushed before
-              // exiting the current batch. So we can skip it here.
-              if (!isPerformingWork) {
-                performWork(TaskPriority);
-              }
+              // TODO: If we're not already performing work, schedule a
+              // deferred callback.
               return;
             case AnimationPriority:
               scheduleAnimationCallback(performAnimationWork);
@@ -1087,7 +1088,8 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   }
 
   function getPriorityContext() : PriorityLevel {
-    if (priorityContext === SynchronousPriority && isPerformingWork) {
+    // If we're in a batch, downgrade sync priority to task priority
+    if (priorityContext === SynchronousPriority && shouldDeferSyncUpdates) {
       return TaskPriority;
     }
     return priorityContext;
@@ -1108,16 +1110,15 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   }
 
   function batchedUpdates<A, R>(fn : (a: A) => R, a : A) : R {
-    const previousIsPerformingWork = isPerformingWork;
-    // Simulate that we're performing work so that sync work is batched
-    isPerformingWork = true;
+    const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
+    shouldDeferSyncUpdates = true;
     try {
       return fn(a);
     } finally {
-      isPerformingWork = previousIsPerformingWork;
-      // If we're not already performing work, we need to flush any task work
+      shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
+      // If we're not already inside a batch, we need to flush any task work
       // that was created by the user-provided function.
-      if (!isPerformingWork) {
+      if (!shouldDeferSyncUpdates) {
         performWork(TaskPriority);
       }
     }
@@ -1125,11 +1126,14 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
 
   function syncUpdates<A>(fn : () => A) : A {
     const previousPriorityContext = priorityContext;
+    const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
     priorityContext = SynchronousPriority;
+    shouldDeferSyncUpdates = false;
     try {
       return fn();
     } finally {
       priorityContext = previousPriorityContext;
+      shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
     }
   }
 

From ce8c978d6305e22bf27ea9754cc4176a0eabd4ef Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Wed, 28 Dec 2016 21:28:40 -0800
Subject: [PATCH 5/6] Disallow forced sync updates during begin phase

Stack allows this, but in Fiber it may cause an infinite loop. Instead
we'll print a warning and defer the update by giving it Task priority.
---
 .../shared/__tests__/ReactPerf-test.js        | 19 ++++++++++++++--
 .../shared/fiber/ReactFiberScheduler.js       | 14 ++++++++++--
 .../ReactIncrementalScheduling-test.js        | 10 +++++++++
 .../__tests__/ReactComponentTreeHook-test.js  | 13 +++++++++++
 .../__tests__/ReactCompositeComponent-test.js |  2 +-
 .../shared/__tests__/ReactUpdates-test.js     | 22 +++++++++++++++++++
 6 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js
index b8f895fc8835..2695e648b2b6 100644
--- a/src/renderers/shared/__tests__/ReactPerf-test.js
+++ b/src/renderers/shared/__tests__/ReactPerf-test.js
@@ -16,6 +16,7 @@ describe('ReactPerf', () => {
   var ReactDOM;
   var ReactPerf;
   var ReactTestUtils;
+  var ReactDOMFeatureFlags;
   var emptyFunction;
 
   var App;
@@ -38,6 +39,7 @@ describe('ReactPerf', () => {
     ReactDOM = require('ReactDOM');
     ReactPerf = require('ReactPerf');
     ReactTestUtils = require('ReactTestUtils');
+    ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
     emptyFunction = require('emptyFunction');
 
     App = class extends React.Component {
@@ -614,7 +616,7 @@ describe('ReactPerf', () => {
       }
     }
     class EvilPortal extends React.Component {
-      componentWillMount() {
+      componentDidMount() {
         var portalContainer = document.createElement('div');
         ReactDOM.render(<Evil />, portalContainer);
       }
@@ -645,6 +647,10 @@ describe('ReactPerf', () => {
     var container = document.createElement('div');
     var thrownErr = new Error('Muhaha!');
 
+    if (ReactDOMFeatureFlags.useFiber) {
+      spyOn(console, 'error');
+    }
+
     class Evil extends React.Component {
       componentWillMount() {
         throw thrownErr;
@@ -679,6 +685,15 @@ describe('ReactPerf', () => {
     }
     ReactDOM.unmountComponentAtNode(container);
     ReactPerf.stop();
+
+    if (ReactDOMFeatureFlags.useFiber) {
+      // A sync `render` inside cWM will print a warning. That should be the
+      // only warning.
+      expect(console.error.calls.count()).toEqual(1);
+      expect(console.error.calls.argsFor(0)[0]).toMatch(
+        /Render methods should be a pure function of props and state/
+      );
+    }
   });
 
   it('should not print errant warnings if portal throws in componentDidMount()', () => {
@@ -694,7 +709,7 @@ describe('ReactPerf', () => {
       }
     }
     class EvilPortal extends React.Component {
-      componentWillMount() {
+      componentDidMount() {
         var portalContainer = document.createElement('div');
         ReactDOM.render(<Evil />, portalContainer);
       }
diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
index 45fc8dbf36ae..e78d1c40ee1c 100644
--- a/src/renderers/shared/fiber/ReactFiberScheduler.js
+++ b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -69,6 +69,7 @@ var {
 if (__DEV__) {
   var ReactFiberInstrumentation = require('ReactFiberInstrumentation');
   var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
+  var warning = require('warning');
 }
 
 var timeHeuristicForUnitOfWork = 1;
@@ -1029,8 +1030,17 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
   }
 
   function scheduleUpdate(fiber : Fiber, priorityLevel : PriorityLevel) {
-    // If we're in a batch, downgrade sync priority to task priority
-    if (priorityLevel === SynchronousPriority && isPerformingWork) {
+    // Detect if a synchronous update is made during render (or begin phase).
+    if (priorityLevel === SynchronousPriority && isPerformingWork && !isCommitting) {
+      if (__DEV__) {
+        warning(
+          false,
+          'Render methods should be a pure function of props and state; ' +
+          'triggering nested component updates from render is not allowed. ' +
+          'If necessary, trigger nested updates in componentDidUpdate.'
+        );
+      }
+      // Downgrade to Task priority to prevent an infinite loop.
       priorityLevel = TaskPriority;
     }
 
diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js
index 26e14b99befe..33a43df6d6f6 100644
--- a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js
+++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js
@@ -340,4 +340,14 @@ describe('ReactIncrementalScheduling', () => {
     // animation priority.
     expect(ReactNoop.getChildren()).toEqual([span(1)]);
   });
+
+  it('can force synchronous updates with syncUpdates, even inside batchedUpdates', done => {
+    ReactNoop.batchedUpdates(() => {
+      ReactNoop.syncUpdates(() => {
+        ReactNoop.render(<span />);
+        expect(ReactNoop.getChildren()).toEqual([span()]);
+        done();
+      });
+    });
+  });
 });
diff --git a/src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js b/src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
index db281517a495..e0114b564314 100644
--- a/src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
+++ b/src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
@@ -18,6 +18,7 @@ describe('ReactComponentTreeHook', () => {
   var ReactInstanceMap;
   var ReactComponentTreeHook;
   var ReactComponentTreeTestUtils;
+  var ReactDOMFeatureFlags;
 
   beforeEach(() => {
     jest.resetModules();
@@ -28,6 +29,7 @@ describe('ReactComponentTreeHook', () => {
     ReactInstanceMap = require('ReactInstanceMap');
     ReactComponentTreeHook = require('ReactComponentTreeHook');
     ReactComponentTreeTestUtils = require('ReactComponentTreeTestUtils');
+    ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
   });
 
   function assertTreeMatches(pairs) {
@@ -1841,6 +1843,9 @@ describe('ReactComponentTreeHook', () => {
       // https://github.com/facebook/react/issues/7187
       var el = document.createElement('div');
       var portalEl = document.createElement('div');
+      if (ReactDOMFeatureFlags.useFiber) {
+        spyOn(console, 'error');
+      }
       class Foo extends React.Component {
         componentWillMount() {
           ReactDOM.render(<div />, portalEl);
@@ -1850,6 +1855,14 @@ describe('ReactComponentTreeHook', () => {
         }
       }
       ReactDOM.render(<Foo />, el);
+      if (ReactDOMFeatureFlags.useFiber) {
+        // A sync `render` inside cWM will print a warning. That should be the
+        // only warning.
+        expect(console.error.calls.count()).toEqual(1);
+        expect(console.error.calls.argsFor(0)[0]).toMatch(
+          /Render methods should be a pure function of props and state/
+        );
+      }
     });
 
     it('is created when calling renderToString during render', () => {
diff --git a/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js
index 1a302d6fcac8..af226d2f2297 100644
--- a/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js
+++ b/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js
@@ -1266,7 +1266,7 @@ describe('ReactCompositeComponent', () => {
     var layer = document.createElement('div');
 
     class Component extends React.Component {
-      componentWillMount() {
+      componentDidMount() {
         ReactDOM.render(<div />, layer);
       }
 
diff --git a/src/renderers/shared/shared/__tests__/ReactUpdates-test.js b/src/renderers/shared/shared/__tests__/ReactUpdates-test.js
index 82113a46d314..638ab6eda264 100644
--- a/src/renderers/shared/shared/__tests__/ReactUpdates-test.js
+++ b/src/renderers/shared/shared/__tests__/ReactUpdates-test.js
@@ -1142,4 +1142,26 @@ describe('ReactUpdates', () => {
     expect(container.textContent).toBe('goodbye');
     expect(mounts).toBe(1);
   });
+
+  it('mounts and unmounts are sync even in a batch', () => {
+    var container1 = document.createElement('div');
+    var container2 = document.createElement('div');
+
+    let called = false;
+    class Foo extends React.Component {
+      componentDidMount() {
+        called = true;
+        ReactDOM.render(<div>Hello</div>, container2);
+        expect(container2.textContent).toEqual('Hello');
+        ReactDOM.unmountComponentAtNode(container2);
+        expect(container2.textContent).toEqual('');
+      }
+      render() {
+        return <div>{this.props.step}</div>;
+      }
+    }
+
+    ReactDOM.render(<Foo />, container1);
+    expect(called).toEqual(true);
+  });
 });

From 79f01b242531bc3706c7f254871eb37e43f60e85 Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Wed, 28 Dec 2016 21:57:34 -0800
Subject: [PATCH 6/6] prepareForCommit returns info object to be passed
 resetAfterCommit

The DOM renderer assumes that resetAfterCommit is called after
prepareForCommit without any nested commits in between. That may not
be the case now that syncUpdates forces a nested update.

To address, this changes the type of prepareForCommit to return a value
which is later passed to resetAfterCommit.
---
 scripts/fiber/tests-passing.txt               |  2 ++
 src/renderers/dom/fiber/ReactDOMFiber.js      | 24 ++++++++++---------
 .../shared/fiber/ReactFiberBeginWork.js       |  4 ++--
 .../shared/fiber/ReactFiberCommitWork.js      |  4 ++--
 .../shared/fiber/ReactFiberCompleteWork.js    |  4 ++--
 .../shared/fiber/ReactFiberHostContext.js     |  4 ++--
 .../shared/fiber/ReactFiberReconciler.js      |  8 +++----
 .../shared/fiber/ReactFiberScheduler.js       |  6 ++---
 8 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
index c56a44b5b28e..c749ed37ab64 100644
--- a/scripts/fiber/tests-passing.txt
+++ b/scripts/fiber/tests-passing.txt
@@ -1207,6 +1207,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js
 * can opt-in to deferred/animation scheduling inside componentDidMount/Update
 * performs Task work even after time runs out
 * does not perform animation work after time runs out
+* can force synchronous updates with syncUpdates, even inside batchedUpdates
 
 src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
 * can update child nodes of a host instance
@@ -1576,6 +1577,7 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js
 * unstable_batchedUpdates should return value from a callback
 * unmounts and remounts a root in the same batch
 * handles reentrant mounting in synchronous mode
+* mounts and unmounts are sync even in a batch
 
 src/renderers/shared/shared/__tests__/refs-destruction-test.js
 * should remove refs when destroying the parent
diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js
index a4ed6d3e547b..26c373490987 100644
--- a/src/renderers/dom/fiber/ReactDOMFiber.js
+++ b/src/renderers/dom/fiber/ReactDOMFiber.js
@@ -67,9 +67,10 @@ type HostContextDev = {
 };
 type HostContextProd = string;
 type HostContext = HostContextDev | HostContextProd;
-
-let eventsEnabled : ?boolean = null;
-let selectionInformation : ?mixed = null;
+type CommitInfo = {
+  eventsEnabled: boolean,
+  selectionInformation: mixed,
+};
 
 var ELEMENT_NODE_TYPE = 1;
 var DOC_NODE_TYPE = 9;
@@ -137,17 +138,18 @@ var DOMRenderer = ReactFiberReconciler({
     return getChildNamespace(parentNamespace, type);
   },
 
-  prepareForCommit() : void {
-    eventsEnabled = ReactBrowserEventEmitter.isEnabled();
+  prepareForCommit() : CommitInfo {
+    const eventsEnabled = ReactBrowserEventEmitter.isEnabled();
     ReactBrowserEventEmitter.setEnabled(false);
-    selectionInformation = ReactInputSelection.getSelectionInformation();
+    return {
+      eventsEnabled,
+      selectionInformation: ReactInputSelection.getSelectionInformation(),
+    };
   },
 
-  resetAfterCommit() : void {
-    ReactInputSelection.restoreSelection(selectionInformation);
-    selectionInformation = null;
-    ReactBrowserEventEmitter.setEnabled(eventsEnabled);
-    eventsEnabled = null;
+  resetAfterCommit(commitInfo : CommitInfo) : void {
+    ReactInputSelection.restoreSelection(commitInfo.selectionInformation);
+    ReactBrowserEventEmitter.setEnabled(commitInfo.eventsEnabled);
   },
 
   createInstance(
diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js
index 4cfb1f4bb1b3..e5ee7a831d18 100644
--- a/src/renderers/shared/fiber/ReactFiberBeginWork.js
+++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js
@@ -66,8 +66,8 @@ if (__DEV__) {
   var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
 }
 
-module.exports = function<T, P, I, TI, C, CX>(
-  config : HostConfig<T, P, I, TI, C, CX>,
+module.exports = function<T, P, I, TI, C, CX, CI>(
+  config : HostConfig<T, P, I, TI, C, CX, CI>,
   hostContext : HostContext<C, CX>,
   scheduleUpdate : (fiber : Fiber, priorityLevel : PriorityLevel) => void,
   getPriorityContext : () => PriorityLevel,
diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js
index bb56a4c78f3b..429e892e0bef 100644
--- a/src/renderers/shared/fiber/ReactFiberCommitWork.js
+++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js
@@ -33,8 +33,8 @@ var {
   ContentReset,
 } = require('ReactTypeOfSideEffect');
 
-module.exports = function<T, P, I, TI, C, CX>(
-  config : HostConfig<T, P, I, TI, C, CX>,
+module.exports = function<T, P, I, TI, C, CX, CI>(
+  config : HostConfig<T, P, I, TI, C, CX, CI>,
   hostContext : HostContext<C, CX>,
   captureError : (failedFiber : Fiber, error: Error) => ?Fiber
 ) {
diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js
index bffecb3c4690..8c836cfda5d2 100644
--- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js
+++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js
@@ -46,8 +46,8 @@ if (__DEV__) {
   var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
 }
 
-module.exports = function<T, P, I, TI, C, CX>(
-  config : HostConfig<T, P, I, TI, C, CX>,
+module.exports = function<T, P, I, TI, C, CX, CI>(
+  config : HostConfig<T, P, I, TI, C, CX, CI>,
   hostContext : HostContext<C, CX>,
 ) {
   const {
diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js
index 529271de6ad3..de311124ca9e 100644
--- a/src/renderers/shared/fiber/ReactFiberHostContext.js
+++ b/src/renderers/shared/fiber/ReactFiberHostContext.js
@@ -34,8 +34,8 @@ export type HostContext<C, CX> = {
   resetHostContainer() : void,
 };
 
-module.exports = function<T, P, I, TI, C, CX>(
-  config : HostConfig<T, P, I, TI, C, CX>
+module.exports = function<T, P, I, TI, C, CX, CI>(
+  config : HostConfig<T, P, I, TI, C, CX, CI>
 ) : HostContext<C, CX> {
   const {
     getChildHostContext,
diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js
index ff55cbddaa00..3424ef683faa 100644
--- a/src/renderers/shared/fiber/ReactFiberReconciler.js
+++ b/src/renderers/shared/fiber/ReactFiberReconciler.js
@@ -43,7 +43,7 @@ export type Deadline = {
 
 type OpaqueNode = Fiber;
 
-export type HostConfig<T, P, I, TI, C, CX> = {
+export type HostConfig<T, P, I, TI, C, CX, CI> = {
 
   getRootHostContext(rootContainerInstance : C) : CX,
   getChildHostContext(parentHostContext : CX, type : T) : CX,
@@ -69,8 +69,8 @@ export type HostConfig<T, P, I, TI, C, CX> = {
   scheduleAnimationCallback(callback : () => void) : void,
   scheduleDeferredCallback(callback : (deadline : Deadline) => void) : void,
 
-  prepareForCommit() : void,
-  resetAfterCommit() : void,
+  prepareForCommit() : CI,
+  resetAfterCommit(commitInfo : CI) : void,
 
   useSyncScheduling ?: boolean,
 };
@@ -100,7 +100,7 @@ getContextForSubtree._injectFiber(function(fiber : Fiber) {
     parentContext;
 });
 
-module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C, CX>) : Reconciler<C, I, TI> {
+module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, TI, C, CX, CI>) : Reconciler<C, I, TI> {
 
   var {
     scheduleUpdate,
diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
index e78d1c40ee1c..20603ec6889e 100644
--- a/src/renderers/shared/fiber/ReactFiberScheduler.js
+++ b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -74,7 +74,7 @@ if (__DEV__) {
 
 var timeHeuristicForUnitOfWork = 1;
 
-module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C, CX>) {
+module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, TI, C, CX, CI>) {
   const hostContext = ReactFiberHostContext(config);
   const { popHostContainer, popHostContext, resetHostContainer } = hostContext;
   const { beginWork, beginFailedWork } = ReactFiberBeginWork(
@@ -348,7 +348,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
       firstEffect = finishedWork.firstEffect;
     }
 
-    prepareForCommit();
+    const commitInfo = prepareForCommit();
 
     // Commit all the side-effects within a tree. We'll do this in two passes.
     // The first pass performs all the host insertions, updates, deletions and
@@ -369,7 +369,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
       }
     }
 
-    resetAfterCommit();
+    resetAfterCommit(commitInfo);
 
     // In the second pass we'll perform all life-cycles and ref callbacks.
     // Life-cycles happen as a separate pass so that all placements, updates,