Emit warning when props parsing encounters inconsistent duplicate keys (

facebook#39149)

Summary:
Pull Request resolved: facebook#39149

Props structs are allowed to references RawValue keys multiple times, however this didn't work correctly if the key was formed out of a different combination of prefix/name/suffix, as the duplicate key entry would've been removed from the `RawPropsKeyMap`. Log an error in such scenarios to help identify these types of problems.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D48641819

fbshipit-source-id: 6b68480b0646a5598de2413de30aa817f354f2fc

packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.cpp

@@ -9,6 +9,7 @@
 
 #include <react/debug/react_native_assert.h>
 
+#include <glog/logging.h>
 #include <algorithm>
 #include <cassert>
 #include <cstdlib>
@@ -51,15 +52,26 @@ void RawPropsKeyMap::reindex() noexcept {
       &RawPropsKeyMap::shouldFirstOneBeBeforeSecondOne);
 
   // Filtering out duplicating keys.
-  // If some `*Props` object requests a prop more than once, only the first
-  // request will be fulfilled. E.g. `TextInputProps` class has a sub-property
-  // `backgroundColor` twice, the first time as part of `ViewProps` base-class
-  // and the second as part of `BaseTextProps` base-class. In this
-  // configuration, the only one which comes first (from `ViewProps`, which
-  // appear first) will be assigned.
-  items_.erase(
-      std::unique(items_.begin(), items_.end(), &RawPropsKeyMap::hasSameName),
-      items_.end());
+  // Accessing the same key twice is supported by RawPropsPorser, but the
+  // RawPropsKey used must be identical, and if not, lookup will be
+  // inconsistent.
+  auto it = items_.begin();
+  auto end = items_.end();
+  // Implements std::unique with additional logging
+  if (it != end) {
+    auto result = it;
+    while (++it != end) {
+      if (hasSameName(*result, *it)) {
+        LOG(ERROR)
+            << "Component property map contains multiple entries for '"
+            << std::string_view(it->name, it->length)
+            << "'. Ensure all calls to convertRawProp use a consistent prefix, name and suffix.";
+      } else if (++result != it) {
+        *result = *it;
+      }
+    }
+    items_.erase(++result, items_.end());
+  }
 
   buckets_.resize(kPropNameLengthHardCap);
 

packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp

@@ -52,18 +52,18 @@ RawValue const *RawPropsParser::at(
   // index, there's no need to do any lookups. However, it's possible for keys
   // to be accessed out-of-order or multiple times, in which case we start
   // searching again from index 0.
-  // To prevent infinite loops (which can occur if
-  // you look up a key that doesn't exist) we keep track of whether or not we've
-  // already looped around, and log and return nullptr if so. However, we ONLY
-  // do this in debug mode, where you're more likely to look up a nonexistent
-  // key as part of debugging. You can (and must) ensure infinite loops are not
-  // possible in production by: (1) constructing all props objects without
-  // conditionals, or (2) if there are conditionals, ensure that in the parsing
-  // setup case, the Props constructor will access _all_ possible props. To
-  // ensure this performance optimization is utilized, always access props in
-  // the same order every time. This is trivial if you have a simple Props
-  // constructor, but difficult or impossible if you have a shared sub-prop
-  // Struct that is used by multiple parent Props.
+  // To prevent infinite loops (which can occur if you look up a key that
+  // doesn't exist) we keep track of whether or not we've already looped around,
+  // and log and return nullptr if so. However, we ONLY do this in debug mode,
+  // where you're more likely to look up a nonexistent key as part of debugging.
+  // You can (and must) ensure infinite loops are not possible in production by:
+  // (1) constructing all props objects without conditionals, or (2) if there
+  // are conditionals, ensure that in the parsing setup case, the Props
+  // constructor will access _all_ possible props. To ensure this performance
+  // optimization is utilized, always access props in the same order every time.
+  // This is trivial if you have a simple Props constructor, but difficult or
+  // impossible if you have a shared sub-prop Struct that is used by multiple
+  // parent Props.
 #ifdef REACT_NATIVE_DEBUG
   bool resetLoop = false;
 #endif
@@ -73,8 +73,9 @@ RawValue const *RawPropsParser::at(
     if (UNLIKELY(rawProps.keyIndexCursor_ >= keys_.size())) {
 #ifdef REACT_NATIVE_DEBUG
       if (resetLoop) {
-        LOG(ERROR) << "Looked up RawProps key that does not exist: "
-                   << (std::string)key;
+        LOG(ERROR)
+            << "Looked up property name which was not seen when preparing: "
+            << (std::string)key;
         return nullptr;
       }
       resetLoop = true;