From eec431297a145ccc0159ce3a1bf093e47c9c88d5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 13 Dec 2016 15:05:56 -0800 Subject: [PATCH] Priority context during reconciliation setState inside render/cWRP should have the same priority as whatever level is currently being reconciled. --- scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberBeginWork.js | 13 ++- .../shared/fiber/ReactFiberClassComponent.js | 26 ++---- .../shared/fiber/ReactFiberReconciler.js | 10 +-- .../shared/fiber/ReactFiberScheduler.js | 68 ++++++++++++--- .../__tests__/ReactIncrementalUpdates-test.js | 85 ++++++++++--------- 6 files changed, 125 insertions(+), 78 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 91f4eecbc1..5b821222ca 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1237,6 +1237,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js * can abort an update, schedule additional updates, and resume * can abort an update, schedule a replaceState, and resume * does not call callbacks that are scheduled by another callback until a later commit +* gives setState during reconciliation the same priority as whatever level is currently reconciling * enqueues setState inside an updater function as if the in-progress update is progressed (and warns) src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index f00bcca831..94799919b9 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -71,8 +71,10 @@ if (__DEV__) { module.exports = function( config : HostConfig, hostContext : HostContext, - scheduleUpdateAtPriority : (fiber: Fiber, priorityLevel : PriorityLevel) => void, - getPriorityContext : () => PriorityLevel + scheduleSetState: (fiber : Fiber, partialState : any) => void, + scheduleReplaceState: (fiber : Fiber, state : any) => void, + scheduleForceUpdate: (fiber : Fiber) => void, + scheduleUpdateCallback: (fiber : Fiber, callback : Function) => void, ) { const { shouldSetTextContent } = config; @@ -89,7 +91,12 @@ module.exports = function( mountClassInstance, resumeMountClassInstance, updateClassInstance, - } = ReactFiberClassComponent(scheduleUpdateAtPriority, getPriorityContext); + } = ReactFiberClassComponent( + scheduleSetState, + scheduleReplaceState, + scheduleForceUpdate, + scheduleUpdateCallback + ); function markChildAsProgressed(current, workInProgress, priorityLevel) { // We now have clones. Let's store them as the currently progressed work. diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 92179a0806..1852b52aba 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -19,10 +19,6 @@ var { getMaskedContext, } = require('ReactFiberContext'); var { - addUpdate, - addReplaceUpdate, - addForceUpdate, - addCallback, beginUpdateQueue, } = require('ReactFiberUpdateQueue'); var { hasContextChanged } = require('ReactFiberContext'); @@ -36,8 +32,10 @@ var invariant = require('invariant'); const isArray = Array.isArray; module.exports = function( - scheduleUpdateAtPriority : (fiber: Fiber, priorityLevel : PriorityLevel) => void, - getPriorityContext : () => PriorityLevel, + scheduleSetState: (fiber : Fiber, partialState : any) => void, + scheduleReplaceState: (fiber : Fiber, state : any) => void, + scheduleForceUpdate: (fiber : Fiber) => void, + scheduleUpdateCallback: (fiber : Fiber, callback : Function) => void, ) { // Class component state updater @@ -45,27 +43,19 @@ module.exports = function( isMounted, enqueueSetState(instance, partialState) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addUpdate(fiber, partialState, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleSetState(fiber, partialState); }, enqueueReplaceState(instance, state) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addReplaceUpdate(fiber, state, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleReplaceState(fiber, state); }, enqueueForceUpdate(instance) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addForceUpdate(fiber, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleForceUpdate(fiber); }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addCallback(fiber, callback, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleUpdateCallback(fiber, callback); }, }; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 47f371b5e4..177fc674dc 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -24,8 +24,6 @@ var { var { createFiberRoot } = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); -var { addCallback } = require('ReactFiberUpdateQueue'); - if (__DEV__) { var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); } @@ -100,7 +98,7 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig) { const hostContext = ReactFiberHostContext(config); const { popHostContainer, popHostContext, resetHostContainer } = hostContext; - const { beginWork, beginFailedWork } = - ReactFiberBeginWork(config, hostContext, scheduleUpdateAtPriority, getPriorityContext); + const { beginWork, beginFailedWork } = ReactFiberBeginWork( + config, + hostContext, + scheduleSetState, + scheduleReplaceState, + scheduleForceUpdate, + scheduleUpdateCallback, + ); const { completeWork } = ReactFiberCompleteWork(config, hostContext); const { commitPlacement, @@ -95,6 +105,10 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig { ]); }); + it('gives setState during reconciliation the same priority as whatever level is currently reconciling', () => { + let instance; + let ops = []; + + class Foo extends React.Component { + state = {}; + componentWillReceiveProps() { + ops.push('componentWillReceiveProps'); + this.setState({ b: 'b' }); + } + render() { + ops.push('render'); + instance = this; + return ; + } + } + ReactNoop.render(); + ReactNoop.flush(); + + ops = []; + + ReactNoop.performAnimationWork(() => { + instance.setState({ a: 'a' }); + ReactNoop.render(); // Trigger componentWillReceiveProps + }); + ReactNoop.flush(); + + expect(ReactNoop.getChildren()).toEqual([span('ab')]); + expect(ops).toEqual([ + 'componentWillReceiveProps', + 'render', + ]); + }); + + it('enqueues setState inside an updater function as if the in-progress update is progressed (and warns)', () => { spyOn(console, 'error'); let instance; let ops = []; class Foo extends React.Component { state = {}; - componentDidMount() { - ops.push('componentDidMount'); - this.setState(function a() { - // Force update b to have Task priority - ReactNoop.syncUpdates(() => { - this.setState({ b: 'b' }); - }); - return { a: 'a' }; - }); - } render() { ops.push('render'); instance = this; @@ -271,41 +296,25 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.render(); ReactNoop.flush(); - expectDev(console.error.calls.count()).toBe(1); + + instance.setState(function a() { + ops.push('setState updater'); + this.setState({ b: 'b' }); + return { a: 'a' }; + }); + + ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span('ab')]); expect(ops).toEqual([ // Initial render 'render', - 'componentDidMount', - // Updates a and b both have Task priority. Update b is enqueued while - // update a is being processed, but it should be inserted into the queue - // as if update a is already processed. Then processing continues. Because - // they have the same priority, update b is processed in the same batch. - // So there should only be a single render below. + 'setState updater', + // Update b is enqueued with the same priority as update a, so it should + // be flushed in the same commit. 'render', ]); - ops = []; - - ReactNoop.performAnimationWork(() => { - instance.setState(function c() { - // Update d happens during the begin phase, so it has low priority. - this.setState({ d: 'd' }); - return { c: 'c' }; - }); - }); - - ReactNoop.flush(); - expect(ReactNoop.getChildren()).toEqual([span('abcd')]); - expect(ops).toEqual([ - // Update c has animation priority. Update d is enqueued while c is being - // processed with animation priority. Because d is low priority, it is not - // processed until the next render. So there should be two renders below. - 'render', - 'render', - ]); - - expectDev(console.error.calls.count()).toBe(2); + expectDev(console.error.calls.count()).toBe(1); console.error.calls.reset(); }); });