From c4bd7cef69ccdccaee848435ef068bb95c4ca7ba Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 10 Apr 2018 12:46:07 -0700 Subject: [PATCH] Fabric: Fixed issue in the diffing algorithm Summary: Previously we generated `removed` *or* `delete` instruction, but sometimes we have to generate both. So, basically we did it wrong. :( Reviewed By: mdvacca Differential Revision: D7503386 fbshipit-source-id: 8ee476abd29f088f31dc776f6e6a68d5293fbb35 --- .../fabric/uimanager/Differentiator.cpp | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/ReactCommon/fabric/uimanager/Differentiator.cpp b/ReactCommon/fabric/uimanager/Differentiator.cpp index 20ff6add6..87f929904 100644 --- a/ReactCommon/fabric/uimanager/Differentiator.cpp +++ b/ReactCommon/fabric/uimanager/Differentiator.cpp @@ -38,7 +38,6 @@ void calculateMutationInstructions( TreeMutationInstructionList downwardInstructions = {}; // Stage 1: Collectings Updates - for (index = 0; index < oldChildNodes->size() && index < newChildNodes->size(); index++) { SharedShadowNode oldChildNode = oldChildNodes->at(index); SharedShadowNode newChildNode = newChildNodes->at(index); @@ -76,7 +75,7 @@ void calculateMutationInstructions( insertInstructions.push_back( TreeMutationInstruction::Insert( parentNode, - newChildNodes->at(index), + newChildNode, index ) ); @@ -99,20 +98,20 @@ void calculateMutationInstructions( for (index = lastIndexAfterFirstStage; index < oldChildNodes->size(); index++) { SharedShadowNode oldChildNode = oldChildNodes->at(index); + // Even if the old node was (re)inserted, we have to generate `remove` + // instruction. + removeInstructions.push_back( + TreeMutationInstruction::Remove( + parentNode, + oldChildNode, + index + ) + ); + auto numberOfRemovedTags = insertedTags.erase(oldChildNode->getTag()); assert(numberOfRemovedTags == 0 || numberOfRemovedTags == 1); - if (numberOfRemovedTags != 0) { - // The old node *was* (re)inserted, - // so we have to generate `remove` instruction. - removeInstructions.push_back( - TreeMutationInstruction::Remove( - parentNode, - oldChildNode, - index - ) - ); - } else { + if (numberOfRemovedTags == 0) { // The old node was *not* (re)inserted, // so we have to generate `delete` instruction and apply the algorithm // recursively.