Fabric: ComponentDescriptor::cloneProps() now never returns the bas…

…e props objects

Summary:
The diff changes how the `empty raw props` optimization works in `ComponentDescriptor::cloneProps()`. Now it only fires only when the base `props` object is null, which is practically all production cases we have (and care about). (I tried, in a normal run there were no cases where the empty raw props were passed with non-null props.) From the other side, the old behavior that may return the same props objects previously several times created bugs and practically unexpected results and practically disallowed to clone props objects easily.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D21110608

fbshipit-source-id: 884807cd8e9c5c3e6cc1c9e4c1f0227259cc21fb

ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h

@@ -98,6 +98,7 @@ class ComponentDescriptor {
    * `props` and `rawProps` applied on top of this.
    * If `props` is `nullptr`, a default `Props` object (with default values)
    * will be used.
+   * Must return an object which is NOT pointer equal to `props`.
    */
   virtual SharedProps cloneProps(
       const SharedProps &props,

ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h

@@ -108,8 +108,13 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
         dynamic_cast<ConcreteProps const *>(props.get()) &&
             "Provided `props` has an incompatible type.");
 
-    if (rawProps.isEmpty()) {
-      return props ? props : ShadowNodeT::defaultSharedProps();
+    // Optimization:
+    // Quite often nodes are constructed with default/empty props: the base
+    // `props` object is `null` (there no base because it's not cloning) and the
+    // `rawProps` is empty. In this case, we can return the default props object
+    // of a concrete type entirely bypassing parsing.
+    if (!props && rawProps.isEmpty()) {
+      return ShadowNodeT::defaultSharedProps();
     }
 
     rawProps.parse(rawPropsParser_);

ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp

@@ -72,6 +72,9 @@ TEST(ComponentDescriptorTest, cloneShadowNode) {
   EXPECT_EQ(cloned->getTag(), 9);
   EXPECT_EQ(cloned->getSurfaceId(), 1);
   EXPECT_STREQ(cloned->getProps()->nativeId.c_str(), "abc");
+
+  auto clonedButSameProps = descriptor->cloneProps(props, RawProps());
+  EXPECT_NE(clonedButSameProps, props);
 }
 
 TEST(ComponentDescriptorTest, appendChild) {