From 3f63c723b7cd8a373039cb5d338ec6f644bbb1d3 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 7 Jun 2017 21:07:32 +0100 Subject: [PATCH 1/2] Add Bailout effectTag for React DevTools --- .../shared/fiber/ReactFiberClassComponent.js | 20 +++++++++++------- .../shared/fiber/ReactFiberScheduler.js | 12 +++++++---- .../shared/fiber/ReactTypeOfSideEffect.js | 21 +++++++++++-------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 1d695f7cb2e..b7d74db244f 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -15,7 +15,7 @@ import type {Fiber} from 'ReactFiber'; import type {PriorityLevel} from 'ReactPriorityLevel'; -var {Update} = require('ReactTypeOfSideEffect'); +var {Bailout, Update} = require('ReactTypeOfSideEffect'); var ReactFeatureFlags = require('ReactFeatureFlags'); var {AsyncUpdates} = require('ReactTypeOfInternalContext'); @@ -620,13 +620,17 @@ module.exports = function( } else { // If an update was already in progress, we should schedule an Update // effect even though we're bailing out, so that cWU/cDU are called. - if (typeof instance.componentDidUpdate === 'function') { - if ( - oldProps !== current.memoizedProps || - oldState !== current.memoizedState - ) { - workInProgress.effectTag |= Update; - } + if ( + typeof instance.componentDidUpdate === 'function' && + (oldProps !== current.memoizedProps || + oldState !== current.memoizedState) + ) { + workInProgress.effectTag |= Update | Bailout; + } else { + // Also mark it as a bailout. If alone, it won't be part of the effect list, + // but React DevTools will be able to determine this Fiber hasn't updated, + // and avoid flashing the component in "Highlight Updates" tracing mode. + workInProgress.effectTag |= Bailout; } // If shouldComponentUpdate returned false, we should still update the diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 8b02594eee7..188f802b30c 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -64,7 +64,7 @@ var { var {AsyncUpdates} = require('ReactTypeOfInternalContext'); var { - NoEffect, + Bailout, Placement, Update, PlacementAndUpdate, @@ -335,7 +335,8 @@ module.exports = function( // updates, and deletions. To avoid needing to add a case for every // possible bitmap value, we remove the secondary effects from the // effect tag and switch on that value. - let primaryEffectTag = effectTag & ~(Callback | Err | ContentReset | Ref); + let primaryEffectTag = + effectTag & ~(Callback | Err | ContentReset | Ref | Bailout); switch (primaryEffectTag) { case Placement: { commitPlacement(nextEffect); @@ -445,7 +446,7 @@ module.exports = function( priorityContext = TaskPriority; let firstEffect; - if (finishedWork.effectTag !== NoEffect) { + if (finishedWork.effectTag > Bailout) { // A fiber's effect list consists only of its children, not itself. So if // the root has an effect, we need to add it to the end of the list. The // resulting list is the set that would belong to the root's parent, if @@ -641,7 +642,10 @@ module.exports = function( // to schedule our own side-effect on our own list because if end up // reusing children we'll schedule this effect onto itself since we're // at the end. - if (workInProgress.effectTag !== NoEffect) { + const effectTag = workInProgress.effectTag; + // Skip both NoWork and Bailout tags when creating the effect list. + // Bailout effect is read by React DevTools but shouldn't be committed. + if (effectTag > Bailout) { if (returnFiber.lastEffect !== null) { returnFiber.lastEffect.nextEffect = workInProgress; } else { diff --git a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js index 408d1d5001c..08b1a0a6f2b 100644 --- a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js +++ b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js @@ -15,13 +15,16 @@ export type TypeOfSideEffect = number; module.exports = { - NoEffect: 0, // 0b0000000 - Placement: 1, // 0b0000001 - Update: 2, // 0b0000010 - PlacementAndUpdate: 3, // 0b0000011 - Deletion: 4, // 0b0000100 - ContentReset: 8, // 0b0001000 - Callback: 16, // 0b0010000 - Err: 32, // 0b0100000 - Ref: 64, // 0b1000000 + // Don't change these two values: + NoEffect: 0, // 0b00000000 + Bailout: 1, // 0b00000001 + // You can change the rest (and add more). + Placement: 2, // 0b00000010 + Update: 4, // 0b00000100 + PlacementAndUpdate: 6, // 0b00000110 + Deletion: 8, // 0b00001000 + ContentReset: 16, // 0b00010000 + Callback: 32, // 0b00100000 + Err: 64, // 0b01000000 + Ref: 128, // 0b10000000 }; From a3b44fffcd6bd5cd3890b7262f1209a6f960e337 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 8 Jun 2017 20:57:01 +0100 Subject: [PATCH 2/2] Instead of tracking bailout, track performed work --- .../shared/fiber/ReactFiberBeginWork.js | 14 ++++++++++++- .../shared/fiber/ReactFiberClassComponent.js | 20 ++++++++----------- .../shared/fiber/ReactFiberScheduler.js | 12 +++++------ .../shared/fiber/ReactTypeOfSideEffect.js | 2 +- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 614dd92020e..23d7898b8a8 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -50,7 +50,13 @@ var { Fragment, } = ReactTypeOfWork; var {NoWork, OffscreenPriority} = require('ReactPriorityLevel'); -var {Placement, ContentReset, Err, Ref} = require('ReactTypeOfSideEffect'); +var { + PerformedWork, + Placement, + ContentReset, + Err, + Ref, +} = require('ReactTypeOfSideEffect'); var ReactFiberClassComponent = require('ReactFiberClassComponent'); var {ReactCurrentOwner} = require('ReactGlobalSharedState'); var invariant = require('fbjs/lib/invariant'); @@ -249,6 +255,8 @@ module.exports = function( } else { nextChildren = fn(nextProps, context); } + // React DevTools reads this flag. + workInProgress.effectTag |= PerformedWork; reconcileChildren(current, workInProgress, nextChildren); memoizeProps(workInProgress, nextProps); return workInProgress.child; @@ -315,6 +323,8 @@ module.exports = function( } else { nextChildren = instance.render(); } + // React DevTools reads this flag. + workInProgress.effectTag |= PerformedWork; reconcileChildren(current, workInProgress, nextChildren); // Memoize props and state using the values we just used to render. // TODO: Restructure so we never read values from the instance. @@ -548,6 +558,8 @@ module.exports = function( } else { value = fn(props, context); } + // React DevTools reads this flag. + workInProgress.effectTag |= PerformedWork; if ( typeof value === 'object' && diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index b7d74db244f..1d695f7cb2e 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -15,7 +15,7 @@ import type {Fiber} from 'ReactFiber'; import type {PriorityLevel} from 'ReactPriorityLevel'; -var {Bailout, Update} = require('ReactTypeOfSideEffect'); +var {Update} = require('ReactTypeOfSideEffect'); var ReactFeatureFlags = require('ReactFeatureFlags'); var {AsyncUpdates} = require('ReactTypeOfInternalContext'); @@ -620,17 +620,13 @@ module.exports = function( } else { // If an update was already in progress, we should schedule an Update // effect even though we're bailing out, so that cWU/cDU are called. - if ( - typeof instance.componentDidUpdate === 'function' && - (oldProps !== current.memoizedProps || - oldState !== current.memoizedState) - ) { - workInProgress.effectTag |= Update | Bailout; - } else { - // Also mark it as a bailout. If alone, it won't be part of the effect list, - // but React DevTools will be able to determine this Fiber hasn't updated, - // and avoid flashing the component in "Highlight Updates" tracing mode. - workInProgress.effectTag |= Bailout; + if (typeof instance.componentDidUpdate === 'function') { + if ( + oldProps !== current.memoizedProps || + oldState !== current.memoizedState + ) { + workInProgress.effectTag |= Update; + } } // If shouldComponentUpdate returned false, we should still update the diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 188f802b30c..e37d010f0bc 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -64,7 +64,7 @@ var { var {AsyncUpdates} = require('ReactTypeOfInternalContext'); var { - Bailout, + PerformedWork, Placement, Update, PlacementAndUpdate, @@ -336,7 +336,7 @@ module.exports = function( // possible bitmap value, we remove the secondary effects from the // effect tag and switch on that value. let primaryEffectTag = - effectTag & ~(Callback | Err | ContentReset | Ref | Bailout); + effectTag & ~(Callback | Err | ContentReset | Ref | PerformedWork); switch (primaryEffectTag) { case Placement: { commitPlacement(nextEffect); @@ -446,7 +446,7 @@ module.exports = function( priorityContext = TaskPriority; let firstEffect; - if (finishedWork.effectTag > Bailout) { + if (finishedWork.effectTag > PerformedWork) { // A fiber's effect list consists only of its children, not itself. So if // the root has an effect, we need to add it to the end of the list. The // resulting list is the set that would belong to the root's parent, if @@ -643,9 +643,9 @@ module.exports = function( // reusing children we'll schedule this effect onto itself since we're // at the end. const effectTag = workInProgress.effectTag; - // Skip both NoWork and Bailout tags when creating the effect list. - // Bailout effect is read by React DevTools but shouldn't be committed. - if (effectTag > Bailout) { + // Skip both NoWork and PerformedWork tags when creating the effect list. + // PerformedWork effect is read by React DevTools but shouldn't be committed. + if (effectTag > PerformedWork) { if (returnFiber.lastEffect !== null) { returnFiber.lastEffect.nextEffect = workInProgress; } else { diff --git a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js index 08b1a0a6f2b..b98f71cc4a2 100644 --- a/src/renderers/shared/fiber/ReactTypeOfSideEffect.js +++ b/src/renderers/shared/fiber/ReactTypeOfSideEffect.js @@ -17,7 +17,7 @@ export type TypeOfSideEffect = number; module.exports = { // Don't change these two values: NoEffect: 0, // 0b00000000 - Bailout: 1, // 0b00000001 + PerformedWork: 1, // 0b00000001 // You can change the rest (and add more). Placement: 2, // 0b00000010 Update: 4, // 0b00000100