Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Emit warning when props parsing encounters inconsistent duplicate keys (#39149)

Browse Source 
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/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: 8cf71366cbf8ae6bee418a6c9d030ebd38784d52
This commit is contained in:
Pieter De Baets
2023-08-25 05:13:39 -07:00
committed by Facebook GitHub Bot
parent a05858680a
commit 3c87455c12
2 changed files with 36 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.cpp
+21 -9
View File
@@ -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
+15 -14
View File
@@ -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;