Merge pull request #8661 from acdlite/fiberunbatchedupdates

[Fiber] Introduce API to opt-out of batching

Author: acdlite

scripts/fiber/tests-passing.txt

@@ -1207,7 +1207,8 @@ 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
+* can opt-out of batching using unbatchedUpdates
+* nested updates are always deferred, even inside unbatchedUpdates
 
 src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
 * can update child nodes of a host instance

src/renderers/dom/fiber/ReactDOMFiber.js

@@ -326,8 +326,8 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,
     }
     const newRoot = DOMRenderer.createContainer(container);
     root = container._reactRootContainer = newRoot;
-    // Initial mount is always sync, even if we're in a batch.
-    DOMRenderer.syncUpdates(() => {
+    // Initial mount should not be batched.
+    DOMRenderer.unbatchedUpdates(() => {
       DOMRenderer.updateContainer(children, newRoot, parentComponent, callback);
     });
   } else {
@@ -354,8 +354,8 @@ var ReactDOM = {
   unmountComponentAtNode(container : DOMContainerElement) {
     warnAboutUnstableUse();
     if (container._reactRootContainer) {
-      // Unmount is always sync, even if we're in a batch.
-      return DOMRenderer.syncUpdates(() => {
+      // Unmount should not be batched.
+      return DOMRenderer.unbatchedUpdates(() => {
         return renderSubtreeIntoContainer(null, null, container, () => {
           container._reactRootContainer = null;
         });

src/renderers/noop/ReactNoop.js

@@ -264,6 +264,8 @@ var ReactNoop = {
 
   batchedUpdates: NoopRenderer.batchedUpdates,
 
+  unbatchedUpdates: NoopRenderer.unbatchedUpdates,
+
   syncUpdates: NoopRenderer.syncUpdates,
 
   // Logs the current state of the tree.

src/renderers/shared/__tests__/ReactPerf-test.js

@@ -16,7 +16,6 @@ describe('ReactPerf', () => {
   var ReactDOM;
   var ReactPerf;
   var ReactTestUtils;
-  var ReactDOMFeatureFlags;
   var emptyFunction;
 
   var App;
@@ -39,7 +38,6 @@ describe('ReactPerf', () => {
     ReactDOM = require('ReactDOM');
     ReactPerf = require('ReactPerf');
     ReactTestUtils = require('ReactTestUtils');
-    ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
     emptyFunction = require('emptyFunction');
 
     App = class extends React.Component {
@@ -647,10 +645,6 @@ 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;
@@ -685,15 +679,6 @@ 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()', () => {

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -82,6 +82,7 @@ export type Reconciler<C, I, TI> = {
   /* eslint-disable no-undef */
   // FIXME: ESLint complains about type parameter
   batchedUpdates<A>(fn : () => A) : A,
+  unbatchedUpdates<A>(fn : () => A) : A,
   syncUpdates<A>(fn : () => A) : A,
   deferredUpdates<A>(fn : () => A) : A,
   /* eslint-enable no-undef */
@@ -107,6 +108,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
     getPriorityContext,
     performWithPriority,
     batchedUpdates,
+    unbatchedUpdates,
     syncUpdates,
     deferredUpdates,
   } = ReactFiberScheduler(config);
@@ -161,6 +163,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
 
     batchedUpdates,
 
+    unbatchedUpdates,
+
     syncUpdates,
 
     deferredUpdates,

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -69,7 +69,6 @@ var {
 if (__DEV__) {
   var ReactFiberInstrumentation = require('ReactFiberInstrumentation');
   var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
-  var warning = require('warning');
 }
 
 var timeHeuristicForUnitOfWork = 1;
@@ -111,8 +110,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
   // 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;
+  // Keeps track of whether we should should batch sync updates.
+  let isBatchingUpdates : boolean = false;
 
   // The next work in progress fiber that we're currently working on.
   let nextUnitOfWork : ?Fiber = null;
@@ -675,31 +674,10 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
   }
 
   function performWork(priorityLevel : PriorityLevel, deadline : Deadline | null) {
-    // 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 previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
-    const previousNextEffect = nextEffect;
-    const previousCommitPhaseBoundaries = commitPhaseBoundaries;
-    const previousFirstUncaughtError = firstUncaughtError;
-    const previousFatalError = fatalError;
-    const previousIsCommitting = isCommitting;
-    const previousIsUnmounting = isUnmounting;
-
-    priorityContext = NoWork;
-    priorityContextBeforeReconciliation = NoWork;
+    if (isPerformingWork) {
+      throw new Error('performWork was called recursively.');
+    }
     isPerformingWork = true;
-    shouldDeferSyncUpdates = true;
-    nextEffect = null;
-    commitPhaseBoundaries = null;
-    firstUncaughtError = null;
-    fatalError = null;
-    isCommitting = false;
-    isUnmounting = false;
-
     const isPerformingDeferredWork = Boolean(deadline);
     let deadlineHasExpired = false;
 
@@ -805,17 +783,12 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
 
     const errorToThrow = fatalError || firstUncaughtError;
 
-    // We're done performing work. Restore the previous state of the scheduler.
-    priorityContext = previousPriorityContext;
-    priorityContextBeforeReconciliation = previousPriorityContextBeforeReconciliation;
-    isPerformingWork = previousIsPerformingWork;
-    shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
-    nextEffect = previousNextEffect;
-    commitPhaseBoundaries = previousCommitPhaseBoundaries;
-    firstUncaughtError = previousFirstUncaughtError;
-    fatalError = previousFatalError;
-    isCommitting = previousIsCommitting;
-    isUnmounting = previousIsUnmounting;
+    // We're done performing work. Time to clean up.
+    isPerformingWork = false;
+    fatalError = null;
+    firstUncaughtError = null;
+    capturedErrors = null;
+    failedBoundaries = null;
 
     // It's safe to throw any unhandled errors.
     if (errorToThrow) {
@@ -1030,20 +1003,6 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
   }
 
   function scheduleUpdate(fiber : Fiber, priorityLevel : PriorityLevel) {
-    // 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;
-    }
-
     let node = fiber;
     let shouldContinue = true;
     while (node && shouldContinue) {
@@ -1098,8 +1057,9 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
   }
 
   function getPriorityContext() : PriorityLevel {
-    // If we're in a batch, downgrade sync priority to task priority
-    if (priorityContext === SynchronousPriority && shouldDeferSyncUpdates) {
+    // If we're in a batch, or if we're already performing work, downgrade sync
+    // priority to task priority
+    if (priorityContext === SynchronousPriority && (isPerformingWork || isBatchingUpdates)) {
       return TaskPriority;
     }
     return priorityContext;
@@ -1120,30 +1080,37 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
   }
 
   function batchedUpdates<A, R>(fn : (a: A) => R, a : A) : R {
-    const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
-    shouldDeferSyncUpdates = true;
+    const previousIsBatchingUpdates = isBatchingUpdates;
+    isBatchingUpdates = true;
     try {
       return fn(a);
     } finally {
-      shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
+      isBatchingUpdates = previousIsBatchingUpdates;
       // If we're not already inside a batch, we need to flush any task work
       // that was created by the user-provided function.
-      if (!shouldDeferSyncUpdates) {
+      if (!isPerformingWork && !isBatchingUpdates) {
         performWork(TaskPriority);
       }
     }
   }
 
+  function unbatchedUpdates<A>(fn : () => A) : A {
+    const previousIsBatchingUpdates = isBatchingUpdates;
+    isBatchingUpdates = false;
+    try {
+      return fn();
+    } finally {
+      isBatchingUpdates = previousIsBatchingUpdates;
+    }
+  }
+
   function syncUpdates<A>(fn : () => A) : A {
     const previousPriorityContext = priorityContext;
-    const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
     priorityContext = SynchronousPriority;
-    shouldDeferSyncUpdates = false;
     try {
       return fn();
     } finally {
       priorityContext = previousPriorityContext;
-      shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
     }
   }
 
@@ -1162,6 +1129,7 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
     getPriorityContext: getPriorityContext,
     performWithPriority: performWithPriority,
     batchedUpdates: batchedUpdates,
+    unbatchedUpdates: unbatchedUpdates,
     syncUpdates: syncUpdates,
     deferredUpdates: deferredUpdates,
   };

src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js

@@ -341,13 +341,70 @@ describe('ReactIncrementalScheduling', () => {
     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();
+  it('can opt-out of batching using unbatchedUpdates', () => {
+    // syncUpdates gives synchronous priority to updates
+    ReactNoop.syncUpdates(() => {
+      // batchedUpdates downgrades sync updates to task priority
+      ReactNoop.batchedUpdates(() => {
+        ReactNoop.render(<span prop={0} />);
+        expect(ReactNoop.getChildren()).toEqual([]);
+        // Should not have flushed yet because we're still batching
+
+        // unbatchedUpdates reverses the effect of batchedUpdates, so sync
+        // updates are not batched
+        ReactNoop.unbatchedUpdates(() => {
+          ReactNoop.render(<span prop={1} />);
+          expect(ReactNoop.getChildren()).toEqual([span(1)]);
+          ReactNoop.render(<span prop={2} />);
+          expect(ReactNoop.getChildren()).toEqual([span(2)]);
+        });
+
+        ReactNoop.render(<span prop={3} />);
+        expect(ReactNoop.getChildren()).toEqual([span(2)]);
       });
+      // Remaining update is now flushed
+      expect(ReactNoop.getChildren()).toEqual([span(3)]);
     });
   });
+
+  it('nested updates are always deferred, even inside unbatchedUpdates', () => {
+    let instance;
+    let ops = [];
+    class Foo extends React.Component {
+      state = { step: 0 };
+      componentDidUpdate() {
+        ops.push('componentDidUpdate: ' + this.state.step);
+        if (this.state.step === 1) {
+          ReactNoop.unbatchedUpdates(() => {
+            // This is a nested state update, so it should not be
+            // flushed synchronously, even though we wrapped it
+            // in unbatchedUpdates.
+            this.setState({ step: 2 });
+          });
+          expect(ReactNoop.getChildren()).toEqual([span(1)]);
+        }
+      }
+      render() {
+        ops.push('render: ' + this.state.step);
+        instance = this;
+        return <span prop={this.state.step} />;
+      }
+    }
+    ReactNoop.render(<Foo />);
+    ReactNoop.flush();
+    expect(ReactNoop.getChildren()).toEqual([span(0)]);
+
+    ReactNoop.syncUpdates(() => {
+      instance.setState({ step: 1 });
+      expect(ReactNoop.getChildren()).toEqual([span(2)]);
+    });
+
+    expect(ops).toEqual([
+      'render: 0',
+      'render: 1',
+      'componentDidUpdate: 1',
+      'render: 2',
+      'componentDidUpdate: 2',
+    ]);
+  });
 });

src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js

@@ -18,7 +18,6 @@ describe('ReactComponentTreeHook', () => {
   var ReactInstanceMap;
   var ReactComponentTreeHook;
   var ReactComponentTreeTestUtils;
-  var ReactDOMFeatureFlags;
 
   beforeEach(() => {
     jest.resetModules();
@@ -29,7 +28,6 @@ describe('ReactComponentTreeHook', () => {
     ReactInstanceMap = require('ReactInstanceMap');
     ReactComponentTreeHook = require('ReactComponentTreeHook');
     ReactComponentTreeTestUtils = require('ReactComponentTreeTestUtils');
-    ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
   });
 
   function assertTreeMatches(pairs) {
@@ -1843,9 +1841,6 @@ 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);
@@ -1855,14 +1850,6 @@ 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', () => {