Back out "Fabric: Strengthening StubView mutating validation"

Summary: Changelog: [internal] Original commit changeset: 8c8a3d8e205c Reviewed By: ShikaSD Differential Revision: D24951341 fbshipit-source-id: b6109a45f1537a9edc702eafac1736e801fbedc9
facebook · Nov 13, 2020 · 53862a1 · 53862a1
1 parent 58c80d4
commit 53862a1
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 29 deletions.
diff --git a/ReactCommon/react/renderer/mounting/StubView.cpp b/ReactCommon/react/renderer/mounting/StubView.cpp
@@ -10,18 +10,6 @@
 namespace facebook {
 namespace react {
 
-StubView::operator ShadowView() const {
-  auto shadowView = ShadowView{};
-  shadowView.componentName = componentName;
-  shadowView.componentHandle = componentHandle;
-  shadowView.tag = tag;
-  shadowView.props = props;
-  shadowView.eventEmitter = eventEmitter;
-  shadowView.layoutMetrics = layoutMetrics;
-  shadowView.state = state;
-  return shadowView;
-}
-
 void StubView::update(ShadowView const &shadowView) {
   componentName = shadowView.componentName;
   componentHandle = shadowView.componentHandle;

diff --git a/ReactCommon/react/renderer/mounting/StubView.h b/ReactCommon/react/renderer/mounting/StubView.h
@@ -25,8 +25,6 @@ class StubView final {
   StubView() = default;
   StubView(StubView const &stubView) = default;
 
-  operator ShadowView() const;
-
   void update(ShadowView const &shadowView);
 
   ComponentName componentName;

diff --git a/ReactCommon/react/renderer/mounting/StubViewTree.cpp b/ReactCommon/react/renderer/mounting/StubViewTree.cpp
@@ -38,19 +38,30 @@ StubView const &StubViewTree::getRootStubView() const {
   return *registry.at(rootTag);
 }
 
-void StubViewTree::mutate(ShadowViewMutationList const &mutations) {
+/**
+ * ignoreDuplicateCreates: when stubs generates "fake" mutation instructions, in
+ * some cases it can produce too many "create" instructions. We ignore
+ * duplicates and treat them as noops. In the case of verifying actual diffing,
+ * that assert is left on.
+ *
+ * @param mutations
+ * @param ignoreDuplicateCreates
+ */
+void StubViewTree::mutate(
+    ShadowViewMutationList const &mutations,
+    bool ignoreDuplicateCreates) {
   STUB_VIEW_LOG({ LOG(ERROR) << "StubView: Mutating Begin"; });
   for (auto const &mutation : mutations) {
     switch (mutation.type) {
       case ShadowViewMutation::Create: {
         STUB_VIEW_ASSERT(mutation.parentShadowView == ShadowView{});
         STUB_VIEW_ASSERT(mutation.oldChildShadowView == ShadowView{});
-        STUB_VIEW_ASSERT(mutation.newChildShadowView.props);
         auto stubView = std::make_shared<StubView>();
-        stubView->update(mutation.newChildShadowView);
         auto tag = mutation.newChildShadowView.tag;
         STUB_VIEW_LOG({ LOG(ERROR) << "StubView: Create: " << tag; });
-        STUB_VIEW_ASSERT(registry.find(tag) == registry.end());
+        if (!ignoreDuplicateCreates) {
+          STUB_VIEW_ASSERT(registry.find(tag) == registry.end());
+        }
         registry[tag] = stubView;
         break;
       }
@@ -62,9 +73,6 @@ void StubViewTree::mutate(ShadowViewMutationList const &mutations) {
         STUB_VIEW_ASSERT(mutation.newChildShadowView == ShadowView{});
         auto tag = mutation.oldChildShadowView.tag;
         STUB_VIEW_ASSERT(registry.find(tag) != registry.end());
-        auto stubView = registry[tag];
-        STUB_VIEW_ASSERT(
-            (ShadowView)(*stubView) == mutation.oldChildShadowView);
         registry.erase(tag);
         break;
       }
@@ -129,15 +137,19 @@ void StubViewTree::mutate(ShadowViewMutationList const &mutations) {
         STUB_VIEW_LOG({
           LOG(ERROR) << "StubView: Update: " << mutation.newChildShadowView.tag;
         });
-        STUB_VIEW_ASSERT(mutation.newChildShadowView.props);
+
+        // We don't have a strict requirement that oldChildShadowView has any
+        // data. In particular, LayoutAnimations can produce UPDATEs with only a
+        // new node.
         STUB_VIEW_ASSERT(
-            mutation.newChildShadowView.tag == mutation.oldChildShadowView.tag);
+            mutation.newChildShadowView.tag ==
+                mutation.oldChildShadowView.tag ||
+            mutation.oldChildShadowView.tag == 0);
+
         STUB_VIEW_ASSERT(
             registry.find(mutation.newChildShadowView.tag) != registry.end());
-        auto oldStubView = registry[mutation.newChildShadowView.tag];
-        STUB_VIEW_ASSERT(
-            (ShadowView)(*oldStubView) == mutation.oldChildShadowView);
-        oldStubView->update(mutation.newChildShadowView);
+        auto stubView = registry[mutation.newChildShadowView.tag];
+        stubView->update(mutation.newChildShadowView);
         break;
       }
     }

diff --git a/ReactCommon/react/renderer/mounting/StubViewTree.h b/ReactCommon/react/renderer/mounting/StubViewTree.h
@@ -21,7 +21,9 @@ class StubViewTree {
   StubViewTree() = default;
   StubViewTree(ShadowView const &shadowView);
 
-  void mutate(ShadowViewMutationList const &mutations);
+  void mutate(
+      ShadowViewMutationList const &mutations,
+      bool ignoreDuplicateCreates = false);
 
   StubView const &getRootStubView() const;
 

diff --git a/ReactCommon/react/renderer/mounting/stubs.cpp b/ReactCommon/react/renderer/mounting/stubs.cpp
@@ -54,7 +54,7 @@ StubViewTree stubViewTreeFromShadowNode(ShadowNode const &rootShadowNode) {
                          ShadowNode::emptySharedShadowNodeSharedList()});
 
   auto stubViewTree = StubViewTree(ShadowView(*emptyRootShadowNode));
-  stubViewTree.mutate(mutations);
+  stubViewTree.mutate(mutations, true);
   return stubViewTree;
 }