diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c749ed37ab6..c60907f1d22 100644 --- a/scripts/fiber/tests-passing.txt +++ b/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 diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 26c37349098..ed27da5ee98 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -326,8 +326,8 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent { + // 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; }); diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 804aba156fd..b1f2f92a8d9 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/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. diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js index 2695e648b2b..b314d93f81d 100644 --- a/src/renderers/shared/__tests__/ReactPerf-test.js +++ b/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()', () => { diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 3424ef683fa..68052a1db03 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -82,6 +82,7 @@ export type Reconciler = { /* eslint-disable no-undef */ // FIXME: ESLint complains about type parameter batchedUpdates(fn : () => A) : A, + unbatchedUpdates(fn : () => A) : A, syncUpdates(fn : () => A) : A, deferredUpdates(fn : () => A) : A, /* eslint-enable no-undef */ @@ -107,6 +108,7 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(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(fn : () => A) : A { + const previousIsBatchingUpdates = isBatchingUpdates; + isBatchingUpdates = false; + try { + return fn(); + } finally { + isBatchingUpdates = previousIsBatchingUpdates; + } + } + function syncUpdates(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(config : HostConfig { expect(ReactNoop.getChildren()).toEqual([span(1)]); }); - it('can force synchronous updates with syncUpdates, even inside batchedUpdates', done => { - ReactNoop.batchedUpdates(() => { - ReactNoop.syncUpdates(() => { - ReactNoop.render(); - 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(); + 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(); + expect(ReactNoop.getChildren()).toEqual([span(1)]); + ReactNoop.render(); + expect(ReactNoop.getChildren()).toEqual([span(2)]); + }); + + ReactNoop.render(); + 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 ; + } + } + ReactNoop.render(); + 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', + ]); + }); }); diff --git a/src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js b/src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js index e0114b56431..db281517a49 100644 --- a/src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js +++ b/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(
, portalEl); @@ -1855,14 +1850,6 @@ describe('ReactComponentTreeHook', () => { } } ReactDOM.render(, 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__/ReactUpdates-test.js b/src/renderers/shared/shared/__tests__/ReactUpdates-test.js index 638ab6eda26..d353b20b60e 100644 --- a/src/renderers/shared/shared/__tests__/ReactUpdates-test.js +++ b/src/renderers/shared/shared/__tests__/ReactUpdates-test.js @@ -1143,25 +1143,14 @@ describe('ReactUpdates', () => { 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(
Hello
, container2); - expect(container2.textContent).toEqual('Hello'); - ReactDOM.unmountComponentAtNode(container2); - expect(container2.textContent).toEqual(''); - } - render() { - return
{this.props.step}
; - } - } - - ReactDOM.render(, container1); - expect(called).toEqual(true); + it('mounts and unmounts are sync even in a batch', done => { + var container = document.createElement('div'); + ReactDOM.unstable_batchedUpdates(() => { + ReactDOM.render(
Hello
, container); + expect(container.textContent).toEqual('Hello'); + ReactDOM.unmountComponentAtNode(container); + expect(container.textContent).toEqual(''); + done(); + }); }); });