From a98aba4d73a6db2ddb2a44156697adbae429f218 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 6 Oct 2016 18:50:01 -0700 Subject: [PATCH] Always override priority level when visiting children It is not possible for a child to have a higher priority level than we're reconciling at, unless we intentionally want to down-prioritize it. --- src/renderers/shared/fiber/ReactChildFiber.js | 8 ++++---- .../fiber/__tests__/ReactIncremental-test.js | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 28187283fb..96fd9e57fe 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -148,10 +148,10 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { clone.sibling = null; return clone; } else { - if (fiber.pendingWorkPriority === NoWork || - fiber.pendingWorkPriority > priority) { - fiber.pendingWorkPriority = priority; - } + // We override the pending priority even if it is higher, because if + // we're reconciling at a lower priority that means that this was + // down-prioritized. + fiber.pendingWorkPriority = priority; fiber.effectTag = NoWork; fiber.index = 0; fiber.sibling = null; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index a8bfe06f7f..6d4db53996 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -417,6 +417,24 @@ describe('ReactIncremental', () => { // after them which is not correct. ReactNoop.flush(); expect(ops).toEqual(['Bar', 'Middle', 'Bar']); + + ops = []; + + // Let us try this again without fully finishing the first time. This will + // create a hanging subtree that is reconciling at the normal priority. + ReactNoop.render(); + ReactNoop.flushDeferredPri(40); + + expect(ops).toEqual(['Foo', 'Bar']); + + ops = []; + + // This update will create a tree that aborts that work and down-prioritizes + // it. If the priority levels aren't down-prioritized correctly this may + // abort rendering of the down-prioritized content. + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual(['Foo', 'Bar', 'Bar']); }); it('can reuse work done after being preempted', () => {