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.

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);
       }

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;
     }
 

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();
+      });
+    });
+  });
 });

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', () => {

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);
       }
 

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);
+  });
 });