diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c56a44b5b28..c749ed37ab6 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 20d851dcee6..26c37349098 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( @@ -322,9 +324,15 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent { + 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 +354,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; + }); }); } }, diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js index b8f895fc883..2695e648b2b 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(, 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(, portalContainer); } diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 4cfb1f4bb1b..e5ee7a831d1 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( - config : HostConfig, +module.exports = function( + config : HostConfig, hostContext : HostContext, 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 bb56a4c78f3..429e892e0be 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( - config : HostConfig, +module.exports = function( + config : HostConfig, hostContext : HostContext, captureError : (failedFiber : Fiber, error: Error) => ?Fiber ) { diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index bffecb3c469..8c836cfda5d 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( - config : HostConfig, +module.exports = function( + config : HostConfig, hostContext : HostContext, ) { const { diff --git a/src/renderers/shared/fiber/ReactFiberHostContext.js b/src/renderers/shared/fiber/ReactFiberHostContext.js index 529271de6ad..de311124ca9 100644 --- a/src/renderers/shared/fiber/ReactFiberHostContext.js +++ b/src/renderers/shared/fiber/ReactFiberHostContext.js @@ -34,8 +34,8 @@ export type HostContext = { resetHostContainer() : void, }; -module.exports = function( - config : HostConfig +module.exports = function( + config : HostConfig ) : HostContext { const { getChildHostContext, diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index ff55cbddaa0..3424ef683fa 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 = { +export type HostConfig = { getRootHostContext(rootContainerInstance : C) : CX, getChildHostContext(parentHostContext : CX, type : T) : CX, @@ -69,8 +69,8 @@ export type HostConfig = { 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(config : HostConfig) : Reconciler { +module.exports = function(config : HostConfig) : Reconciler { var { scheduleUpdate, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 63cea47ee4c..20603ec6889 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -69,11 +69,12 @@ var { if (__DEV__) { var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); + var warning = require('warning'); } var timeHeuristicForUnitOfWork = 1; -module.exports = function(config : HostConfig) { +module.exports = function(config : HostConfig) { const hostContext = ReactFiberHostContext(config); const { popHostContainer, popHostContext, resetHostContainer } = hostContext; const { beginWork, beginFailedWork } = ReactFiberBeginWork( @@ -107,10 +108,12 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(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); } } @@ -1102,11 +1136,14 @@ module.exports = function(config : HostConfig(fn : () => A) : A { const previousPriorityContext = priorityContext; + const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates; priorityContext = SynchronousPriority; + shouldDeferSyncUpdates = false; try { return fn(); } finally { priorityContext = previousPriorityContext; + shouldDeferSyncUpdates = previousShouldDeferSyncUpdates; } } diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js index 26e14b99bef..33a43df6d6f 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(); + 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 db281517a49..e0114b56431 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(
, portalEl); @@ -1850,6 +1855,14 @@ 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__/ReactCompositeComponent-test.js b/src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js index 1a302d6fcac..af226d2f229 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(
, layer); } diff --git a/src/renderers/shared/shared/__tests__/ReactUpdates-test.js b/src/renderers/shared/shared/__tests__/ReactUpdates-test.js index 82113a46d31..638ab6eda26 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(
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); + }); });