From 8c7c2a76b9d497a546609b527a80bd4d195361ba Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Wed, 25 Feb 2026 20:49:02 -0800 Subject: [PATCH] Remove RawPropsKey prefix and suffix Summary: RawPropsKey previously stored three `const char*` fields (prefix, name, suffix) that were concatenated at runtime to form property names. This is pretty niche, used to make a few patterns simpler, but also can lead to confusing conflicts when the same property name can be represented in different ways (e.g. T174300106). Iterator style props parsing also completely avoids it. Lets change the API to a flat name instead. This change is breaking, but could only find a single user (Nitro module) effected, searching through `react-native-libraries`. Changelog: [General][Breaking] - Remove RawPropsKey prefix and suffix Reviewed By: javache Differential Revision: D94367880 --- .../AndroidTextInputProps.cpp | 2 +- .../components/view/AccessibilityProps.cpp | 5 +- .../components/view/BaseViewProps.cpp | 62 +++++++++-- .../components/view/propsConversions.h | 101 +++++++++++------- .../react/renderer/core/RawProps.cpp | 9 +- .../react/renderer/core/RawProps.h | 2 +- .../react/renderer/core/RawPropsKey.cpp | 62 ----------- .../react/renderer/core/RawPropsKey.h | 39 ------- .../react/renderer/core/RawPropsKeyMap.cpp | 38 ++----- .../react/renderer/core/RawPropsKeyMap.h | 6 +- .../react/renderer/core/RawPropsParser.cpp | 10 +- .../react/renderer/core/RawPropsParser.h | 6 +- .../react/renderer/core/propsConversions.h | 10 +- .../renderer/core/tests/RawPropsTest.cpp | 96 ++++++++--------- 14 files changed, 188 insertions(+), 260 deletions(-) delete mode 100644 packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.cpp delete mode 100644 packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.h diff --git a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputProps.cpp index 5b88b938f899..31d5e598a697 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputProps.cpp @@ -17,7 +17,7 @@ namespace facebook::react { static bool hasValue(const RawProps& rawProps, bool defaultValue, const char* name) { - auto rawValue = rawProps.at(name, nullptr, nullptr); + auto rawValue = rawProps.at(name); // No change to prop - use default if (rawValue == nullptr) { diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp index cc39827f8eff..fc7ed03d0b97 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp @@ -229,9 +229,8 @@ AccessibilityProps::AccessibilityProps( role = sourceProps.role; accessibilityTraits = sourceProps.accessibilityTraits; } else { - auto* accessibilityRoleValue = - rawProps.at("accessibilityRole", nullptr, nullptr); - auto* roleValue = rawProps.at("role", nullptr, nullptr); + auto* accessibilityRoleValue = rawProps.at("accessibilityRole"); + auto* roleValue = rawProps.at("role"); auto* precedentRoleValue = roleValue != nullptr ? roleValue : accessibilityRoleValue; diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewProps.cpp index 1b3de652825e..c4e8c8171cbf 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/BaseViewProps.cpp @@ -84,8 +84,20 @@ BaseViewProps::BaseViewProps( : convertRawProp( context, rawProps, - "border", - "Radius", + CascadedRectangleCornersNames{ + .topLeft = "borderTopLeftRadius", + .topRight = "borderTopRightRadius", + .bottomLeft = "borderBottomLeftRadius", + .bottomRight = "borderBottomRightRadius", + .topStart = "borderTopStartRadius", + .topEnd = "borderTopEndRadius", + .bottomStart = "borderBottomStartRadius", + .bottomEnd = "borderBottomEndRadius", + .endEnd = "borderEndEndRadius", + .endStart = "borderEndStartRadius", + .startEnd = "borderStartEndRadius", + .startStart = "borderStartStartRadius", + .all = "borderRadius"}, sourceProps.borderRadii, {})), borderColors( @@ -94,8 +106,19 @@ BaseViewProps::BaseViewProps( : convertRawProp( context, rawProps, - "border", - "Color", + CascadedRectangleEdgesNames{ + .left = "borderLeftColor", + .right = "borderRightColor", + .top = "borderTopColor", + .bottom = "borderBottomColor", + .start = "borderStartColor", + .end = "borderEndColor", + .horizontal = "borderHorizontalColor", + .vertical = "borderVerticalColor", + .block = "borderBlockColor", + .blockEnd = "borderBlockEndColor", + .blockStart = "borderBlockStartColor", + .all = "borderColor"}, sourceProps.borderColors, {})), borderCurves( @@ -104,8 +127,20 @@ BaseViewProps::BaseViewProps( : convertRawProp( context, rawProps, - "border", - "Curve", + CascadedRectangleCornersNames{ + .topLeft = "borderTopLeftCurve", + .topRight = "borderTopRightCurve", + .bottomLeft = "borderBottomLeftCurve", + .bottomRight = "borderBottomRightCurve", + .topStart = "borderTopStartCurve", + .topEnd = "borderTopEndCurve", + .bottomStart = "borderBottomStartCurve", + .bottomEnd = "borderBottomEndCurve", + .endEnd = "borderEndEndCurve", + .endStart = "borderEndStartCurve", + .startEnd = "borderStartEndCurve", + .startStart = "borderStartStartCurve", + .all = "borderCurve"}, sourceProps.borderCurves, {})), borderStyles( @@ -114,8 +149,19 @@ BaseViewProps::BaseViewProps( : convertRawProp( context, rawProps, - "border", - "Style", + CascadedRectangleEdgesNames{ + .left = "borderLeftStyle", + .right = "borderRightStyle", + .top = "borderTopStyle", + .bottom = "borderBottomStyle", + .start = "borderStartStyle", + .end = "borderEndStyle", + .horizontal = "borderHorizontalStyle", + .vertical = "borderVerticalStyle", + .block = "borderBlockStyle", + .blockEnd = "borderBlockEndStyle", + .blockStart = "borderBlockStartStyle", + .all = "borderStyle"}, sourceProps.borderStyles, {})), outlineColor( diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/propsConversions.h b/packages/react-native/ReactCommon/react/renderer/components/view/propsConversions.h index 76968bc47fbd..b1ebb4cb4f12 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/propsConversions.h +++ b/packages/react-native/ReactCommon/react/renderer/components/view/propsConversions.h @@ -369,43 +369,67 @@ convertRawProp(const PropsParserContext &context, const RawProps &rawProps, cons return yogaStyle; } +struct CascadedRectangleCornersNames { + const char *topLeft; + const char *topRight; + const char *bottomLeft; + const char *bottomRight; + const char *topStart; + const char *topEnd; + const char *bottomStart; + const char *bottomEnd; + const char *endEnd; + const char *endStart; + const char *startEnd; + const char *startStart; + const char *all; +}; + +struct CascadedRectangleEdgesNames { + const char *left; + const char *right; + const char *top; + const char *bottom; + const char *start; + const char *end; + const char *horizontal; + const char *vertical; + const char *block; + const char *blockEnd; + const char *blockStart; + const char *all; +}; + // This can be deleted when non-iterator ViewProp parsing is deleted template static inline CascadedRectangleCorners convertRawProp( const PropsParserContext &context, const RawProps &rawProps, - const char *prefix, - const char *suffix, + const CascadedRectangleCornersNames &names, const CascadedRectangleCorners &sourceValue, const CascadedRectangleCorners &defaultValue) { CascadedRectangleCorners result; - result.topLeft = - convertRawProp(context, rawProps, "TopLeft", sourceValue.topLeft, defaultValue.topLeft, prefix, suffix); - result.topRight = - convertRawProp(context, rawProps, "TopRight", sourceValue.topRight, defaultValue.topRight, prefix, suffix); + result.topLeft = convertRawProp(context, rawProps, names.topLeft, sourceValue.topLeft, defaultValue.topLeft); + result.topRight = convertRawProp(context, rawProps, names.topRight, sourceValue.topRight, defaultValue.topRight); result.bottomLeft = - convertRawProp(context, rawProps, "BottomLeft", sourceValue.bottomLeft, defaultValue.bottomLeft, prefix, suffix); - result.bottomRight = convertRawProp( - context, rawProps, "BottomRight", sourceValue.bottomRight, defaultValue.bottomRight, prefix, suffix); - - result.topStart = - convertRawProp(context, rawProps, "TopStart", sourceValue.topStart, defaultValue.topStart, prefix, suffix); - result.topEnd = convertRawProp(context, rawProps, "TopEnd", sourceValue.topEnd, defaultValue.topEnd, prefix, suffix); - result.bottomStart = convertRawProp( - context, rawProps, "BottomStart", sourceValue.bottomStart, defaultValue.bottomStart, prefix, suffix); - result.bottomEnd = - convertRawProp(context, rawProps, "BottomEnd", sourceValue.bottomEnd, defaultValue.bottomEnd, prefix, suffix); - result.endEnd = convertRawProp(context, rawProps, "EndEnd", sourceValue.endEnd, defaultValue.endEnd, prefix, suffix); - result.endStart = - convertRawProp(context, rawProps, "EndStart", sourceValue.endStart, defaultValue.endStart, prefix, suffix); - result.startEnd = - convertRawProp(context, rawProps, "StartEnd", sourceValue.startEnd, defaultValue.startEnd, prefix, suffix); + convertRawProp(context, rawProps, names.bottomLeft, sourceValue.bottomLeft, defaultValue.bottomLeft); + result.bottomRight = + convertRawProp(context, rawProps, names.bottomRight, sourceValue.bottomRight, defaultValue.bottomRight); + + result.topStart = convertRawProp(context, rawProps, names.topStart, sourceValue.topStart, defaultValue.topStart); + result.topEnd = convertRawProp(context, rawProps, names.topEnd, sourceValue.topEnd, defaultValue.topEnd); + result.bottomStart = + convertRawProp(context, rawProps, names.bottomStart, sourceValue.bottomStart, defaultValue.bottomStart); + result.bottomEnd = convertRawProp(context, rawProps, names.bottomEnd, sourceValue.bottomEnd, defaultValue.bottomEnd); + result.endEnd = convertRawProp(context, rawProps, names.endEnd, sourceValue.endEnd, defaultValue.endEnd); + result.endStart = convertRawProp(context, rawProps, names.endStart, sourceValue.endStart, defaultValue.endStart); + result.startEnd = convertRawProp(context, rawProps, names.startEnd, sourceValue.startEnd, defaultValue.startEnd); result.startStart = - convertRawProp(context, rawProps, "StartStart", sourceValue.startStart, defaultValue.startStart, prefix, suffix); + convertRawProp(context, rawProps, names.startStart, sourceValue.startStart, defaultValue.startStart); - result.all = convertRawProp(context, rawProps, "", sourceValue.all, defaultValue.all, prefix, suffix); + result.all = convertRawProp(context, rawProps, names.all, sourceValue.all, defaultValue.all); return result; } @@ -414,31 +438,28 @@ template static inline CascadedRectangleEdges convertRawProp( const PropsParserContext &context, const RawProps &rawProps, - const char *prefix, - const char *suffix, + const CascadedRectangleEdgesNames &names, const CascadedRectangleEdges &sourceValue, const CascadedRectangleEdges &defaultValue) { CascadedRectangleEdges result; - result.left = convertRawProp(context, rawProps, "Left", sourceValue.left, defaultValue.left, prefix, suffix); - result.right = convertRawProp(context, rawProps, "Right", sourceValue.right, defaultValue.right, prefix, suffix); - result.top = convertRawProp(context, rawProps, "Top", sourceValue.top, defaultValue.top, prefix, suffix); - result.bottom = convertRawProp(context, rawProps, "Bottom", sourceValue.bottom, defaultValue.bottom, prefix, suffix); + result.left = convertRawProp(context, rawProps, names.left, sourceValue.left, defaultValue.left); + result.right = convertRawProp(context, rawProps, names.right, sourceValue.right, defaultValue.right); + result.top = convertRawProp(context, rawProps, names.top, sourceValue.top, defaultValue.top); + result.bottom = convertRawProp(context, rawProps, names.bottom, sourceValue.bottom, defaultValue.bottom); - result.start = convertRawProp(context, rawProps, "Start", sourceValue.start, defaultValue.start, prefix, suffix); - result.end = convertRawProp(context, rawProps, "End", sourceValue.end, defaultValue.end, prefix, suffix); + result.start = convertRawProp(context, rawProps, names.start, sourceValue.start, defaultValue.start); + result.end = convertRawProp(context, rawProps, names.end, sourceValue.end, defaultValue.end); result.horizontal = - convertRawProp(context, rawProps, "Horizontal", sourceValue.horizontal, defaultValue.horizontal, prefix, suffix); - result.vertical = - convertRawProp(context, rawProps, "Vertical", sourceValue.vertical, defaultValue.vertical, prefix, suffix); - result.block = convertRawProp(context, rawProps, "Block", sourceValue.block, defaultValue.block, prefix, suffix); - result.blockEnd = - convertRawProp(context, rawProps, "BlockEnd", sourceValue.blockEnd, defaultValue.blockEnd, prefix, suffix); + convertRawProp(context, rawProps, names.horizontal, sourceValue.horizontal, defaultValue.horizontal); + result.vertical = convertRawProp(context, rawProps, names.vertical, sourceValue.vertical, defaultValue.vertical); + result.block = convertRawProp(context, rawProps, names.block, sourceValue.block, defaultValue.block); + result.blockEnd = convertRawProp(context, rawProps, names.blockEnd, sourceValue.blockEnd, defaultValue.blockEnd); result.blockStart = - convertRawProp(context, rawProps, "BlockStart", sourceValue.blockStart, defaultValue.blockStart, prefix, suffix); + convertRawProp(context, rawProps, names.blockStart, sourceValue.blockStart, defaultValue.blockStart); - result.all = convertRawProp(context, rawProps, "", sourceValue.all, defaultValue.all, prefix, suffix); + result.all = convertRawProp(context, rawProps, names.all, sourceValue.all, defaultValue.all); return result; } diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp index 66b021d43e34..5a5b405d9516 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp @@ -9,7 +9,6 @@ #include #include -#include #include namespace facebook::react { @@ -110,15 +109,11 @@ bool RawProps::isEmpty() const noexcept { * Returns a const unowning pointer to `RawValue` of a prop with a given name. * Returns `nullptr` if a prop with the given name does not exist. */ -const RawValue* RawProps::at( - const char* name, - const char* prefix, - const char* suffix) const noexcept { +const RawValue* RawProps::at(const char* name) const noexcept { react_native_assert( parser_ && "The object is not parsed. `parse` must be called before `at`."); - return parser_->at( - *this, RawPropsKey{.prefix = prefix, .name = name, .suffix = suffix}); + return parser_->at(*this, name); } } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawProps.h b/packages/react-native/ReactCommon/react/renderer/core/RawProps.h index 631ef315c987..e1ffd6265d35 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawProps.h +++ b/packages/react-native/ReactCommon/react/renderer/core/RawProps.h @@ -90,7 +90,7 @@ class RawProps final { * Returns a const unowning pointer to `RawValue` of a prop with a given name. * Returns `nullptr` if a prop with the given name does not exist. */ - const RawValue *at(const char *name, const char *prefix, const char *suffix) const noexcept; + const RawValue *at(const char *name) const noexcept; private: friend class RawPropsParser; diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.cpp deleted file mode 100644 index 01ac0f16617c..000000000000 --- a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.cpp +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#include "RawPropsKey.h" - -#include -#include -#include - -#include - -namespace facebook::react { - -void RawPropsKey::render(char* buffer, RawPropsPropNameLength* length) - const noexcept { - *length = 0; - - constexpr size_t maxLength = kPropNameLengthHardCap - 1; - - auto appendSegment = [&](const char* segment) { - auto copyLen = std::min(std::strlen(segment), maxLength - *length); - std::memcpy(buffer + *length, segment, copyLen); - *length += static_cast(copyLen); - }; - - if (prefix != nullptr) { - appendSegment(prefix); - } - - appendSegment(name); - - if (suffix != nullptr) { - appendSegment(suffix); - } -} - -RawPropsKey::operator std::string() const noexcept { - auto buffer = std::array(); - RawPropsPropNameLength length = 0; - render(buffer.data(), &length); - return std::string{buffer.data(), length}; -} - -static bool areFieldsEqual(const char* lhs, const char* rhs) { - if (lhs == nullptr || rhs == nullptr) { - return lhs == rhs; - } - return lhs == rhs || strcmp(lhs, rhs) == 0; -} - -bool operator==(const RawPropsKey& lhs, const RawPropsKey& rhs) noexcept { - // Note: We check the name first. - return areFieldsEqual(lhs.name, rhs.name) && - areFieldsEqual(lhs.prefix, rhs.prefix) && - areFieldsEqual(lhs.suffix, rhs.suffix); -} - -} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.h b/packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.h deleted file mode 100644 index fefdf22525d6..000000000000 --- a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKey.h +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include - -#include - -namespace facebook::react { - -/* - * Represent a prop name stored as three `char const *` fragments. - */ -class RawPropsKey final { - public: - const char *prefix{}; - const char *name{}; - const char *suffix{}; - - /* - * Converts to `std::string`. - */ - explicit operator std::string() const noexcept; - - /* - * Renders compound prop name to given buffer and put the resulting length - * into `length`. - */ - void render(char *buffer, RawPropsPropNameLength *length) const noexcept; -}; - -bool operator==(const RawPropsKey &lhs, const RawPropsKey &rhs) noexcept; - -} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.cpp index 81b0113658de..0771a90a073f 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.cpp @@ -9,7 +9,6 @@ #include -#include #include #include #include @@ -33,11 +32,14 @@ bool RawPropsKeyMap::shouldFirstOneBeBeforeSecondOne( } void RawPropsKeyMap::insert( - const RawPropsKey& key, + std::string_view key, RawPropsValueIndex value) noexcept { auto item = Item{}; item.value = value; - key.render(item.name, &item.length); + auto length = + std::min(key.size(), static_cast(kPropNameLengthHardCap - 1)); + std::copy_n(key.data(), length, item.name); + item.length = static_cast(length); items_.push_back(item); react_native_assert( items_.size() < std::numeric_limits::max()); @@ -52,26 +54,9 @@ void RawPropsKeyMap::reindex() noexcept { &RawPropsKeyMap::shouldFirstOneBeBeforeSecondOne); // Filtering out duplicating keys. - // 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(WARNING) - << "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()); - } + items_.erase( + std::unique(items_.begin(), items_.end(), &RawPropsKeyMap::hasSameName), + items_.end()); buckets_.resize(kPropNameLengthHardCap); @@ -91,9 +76,8 @@ void RawPropsKeyMap::reindex() noexcept { } } -RawPropsValueIndex RawPropsKeyMap::at( - const char* name, - RawPropsPropNameLength length) noexcept { +RawPropsValueIndex RawPropsKeyMap::at(std::string_view name) noexcept { + auto length = static_cast(name.size()); react_native_assert(length > 0); react_native_assert(length < kPropNameLengthHardCap); if (length == 0 || length >= kPropNameLengthHardCap) [[unlikely]] { @@ -107,7 +91,7 @@ RawPropsValueIndex RawPropsKeyMap::at( // 2. Binary search in the bucket. while (lower <= upper) { auto median = (lower + upper) / 2; - auto condition = std::memcmp(items_[median].name, name, length); + auto condition = std::memcmp(items_[median].name, name.data(), length); if (condition < 0) { lower = median + 1; } else if (condition == 0) { diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.h b/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.h index ee14ea4ccbb2..15f438af59c3 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.h +++ b/packages/react-native/ReactCommon/react/renderer/core/RawPropsKeyMap.h @@ -7,8 +7,8 @@ #pragma once -#include #include +#include #include namespace facebook::react { @@ -25,7 +25,7 @@ class RawPropsKeyMap final { /* * Stores `value` with by given `key`. */ - void insert(const RawPropsKey &key, RawPropsValueIndex value) noexcept; + void insert(std::string_view key, RawPropsValueIndex value) noexcept; /* * Reindexes the stored data. @@ -37,7 +37,7 @@ class RawPropsKeyMap final { * Finds and returns the `value` (some index) by given `key`. * Returns `kRawPropsValueIndexEmpty` if the value wan't found. */ - RawPropsValueIndex at(const char *name, RawPropsPropNameLength length) noexcept; + RawPropsValueIndex at(std::string_view name) noexcept; private: struct Item { diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp index 71a3fb93fbed..1bf4bf4ae85b 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp @@ -19,7 +19,7 @@ namespace facebook::react { // which props are accessed during parsing, and in which order. const RawValue* RawPropsParser::at( const RawProps& rawProps, - const RawPropsKey& key) const noexcept { + std::string_view key) const noexcept { if (!ready_) [[unlikely]] { // Check against the same key being inserted more than once. // This happens commonly with nested Props structs, where the higher-level @@ -74,7 +74,7 @@ const RawValue* RawPropsParser::at( if (resetLoop) { LOG(ERROR) << "Looked up property name which was not seen when preparing: " - << (std::string)key; + << key; return nullptr; } resetLoop = true; @@ -126,8 +126,7 @@ void RawPropsParser::preparse(const RawProps& rawProps) const noexcept { for (size_t i = 0; i < count; i++) { auto nameValue = names.getValueAtIndex(runtime, i).getString(runtime); auto name = nameValue.utf8(runtime); - auto keyIndex = nameToIndex_.at( - name.data(), static_cast(name.size())); + auto keyIndex = nameToIndex_.at(name); if (keyIndex == kRawPropsValueIndexEmpty) { continue; @@ -150,8 +149,7 @@ void RawPropsParser::preparse(const RawProps& rawProps) const noexcept { for (const auto& pair : dynamic.items()) { auto name = pair.first.getString(); - auto keyIndex = nameToIndex_.at( - name.data(), static_cast(name.size())); + auto keyIndex = nameToIndex_.at(name); if (keyIndex == kRawPropsValueIndexEmpty) { continue; diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.h b/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.h index ad28ae7b2113..c98379b4f833 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.h +++ b/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.h @@ -10,10 +10,10 @@ #include #include #include -#include #include #include #include +#include namespace facebook::react { @@ -70,9 +70,9 @@ class RawPropsParser final { /* * To be used by `RawProps` only. */ - const RawValue *at(const RawProps &rawProps, const RawPropsKey &key) const noexcept; + const RawValue *at(const RawProps &rawProps, std::string_view key) const noexcept; - mutable std::vector keys_{}; + mutable std::vector keys_{}; mutable RawPropsKeyMap nameToIndex_{}; mutable bool ready_{false}; }; diff --git a/packages/react-native/ReactCommon/react/renderer/core/propsConversions.h b/packages/react-native/ReactCommon/react/renderer/core/propsConversions.h index 6148f7f71d62..deacc07289aa 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/propsConversions.h +++ b/packages/react-native/ReactCommon/react/renderer/core/propsConversions.h @@ -11,7 +11,6 @@ #include #include -#include #include namespace facebook::react { @@ -168,11 +167,9 @@ T convertRawProp( const RawProps &rawProps, const char *name, const T &sourceValue, - const U &defaultValue, - const char *namePrefix = nullptr, - const char *nameSuffix = nullptr) + const U &defaultValue) { - const auto *rawValue = rawProps.at(name, namePrefix, nameSuffix); + const auto *rawValue = rawProps.at(name); if (rawValue == nullptr) [[likely]] { return sourceValue; } @@ -189,9 +186,8 @@ T convertRawProp( return result; } catch (const std::exception &e) { // In case of errors, log the error and fall back to the default - RawPropsKey key{.prefix = namePrefix, .name = name, .suffix = nameSuffix}; // TODO: report this using ErrorUtils so it's more visible to the user - LOG(ERROR) << "Error while converting prop '" << static_cast(key) << "': " << e.what(); + LOG(ERROR) << "Error while converting prop '" << name << "': " << e.what(); return defaultValue; } } diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/RawPropsTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/RawPropsTest.cpp index c905cd1e84ec..caff5c160c32 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/RawPropsTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/RawPropsTest.cpp @@ -175,7 +175,7 @@ TEST(RawPropsTest, handleRawPropsSingleString) { parser.prepare(); raw.parse(parser); - std::string value = (std::string)*raw.at("nativeID", nullptr, nullptr); + std::string value = (std::string)*raw.at("nativeID"); EXPECT_STREQ(value.c_str(), "abc"); } @@ -189,7 +189,7 @@ TEST(RawPropsTest, handleRawPropsSingleFloat) { parser.prepare(); raw.parse(parser); - auto value = (float)*raw.at("floatValue", nullptr, nullptr); + auto value = (float)*raw.at("floatValue"); EXPECT_NEAR(value, 42.42, 0.00001); } @@ -203,7 +203,7 @@ TEST(RawPropsTest, handleRawPropsSingleDouble) { parser.prepare(); raw.parse(parser); - auto value = (double)*raw.at("doubleValue", nullptr, nullptr); + auto value = (double)*raw.at("doubleValue"); EXPECT_NEAR(value, 42.42, 0.00001); } @@ -217,7 +217,7 @@ TEST(RawPropsTest, handleRawPropsSingleInt) { parser.prepare(); raw.parse(parser); - int value = (int)*raw.at("intValue", nullptr, nullptr); + int value = (int)*raw.at("intValue"); EXPECT_EQ(value, 42); } @@ -231,9 +231,9 @@ TEST(RawPropsTest, handleRawPropsSingleIntGetManyTimes) { parser.prepare(); raw.parse(parser); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_EQ((int)*raw.at("intValue"), 42); } TEST(RawPropsTest, handleRawPropsPrimitiveTypes) { @@ -249,13 +249,11 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypes) { parser.prepare(); raw.parse(parser); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_NEAR((double)*raw.at("doubleValue", nullptr, nullptr), 17.42, 0.0001); - EXPECT_NEAR((float)*raw.at("floatValue", nullptr, nullptr), 66.67, 0.00001); - EXPECT_STREQ( - ((std::string)*raw.at("stringValue", nullptr, nullptr)).c_str(), - "helloworld"); - EXPECT_EQ((bool)*raw.at("boolValue", nullptr, nullptr), true); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_NEAR((double)*raw.at("doubleValue"), 17.42, 0.0001); + EXPECT_NEAR((float)*raw.at("floatValue"), 66.67, 0.00001); + EXPECT_STREQ(((std::string)*raw.at("stringValue")).c_str(), "helloworld"); + EXPECT_EQ((bool)*raw.at("boolValue"), true); } TEST(RawPropsTest, handleRawPropsPrimitiveTypesGetTwice) { @@ -271,21 +269,17 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesGetTwice) { parser.prepare(); raw.parse(parser); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_NEAR((double)*raw.at("doubleValue", nullptr, nullptr), 17.42, 0.0001); - EXPECT_NEAR((float)*raw.at("floatValue", nullptr, nullptr), 66.67, 0.00001); - EXPECT_STREQ( - ((std::string)*raw.at("stringValue", nullptr, nullptr)).c_str(), - "helloworld"); - EXPECT_EQ((bool)*raw.at("boolValue", nullptr, nullptr), true); - - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_NEAR((double)*raw.at("doubleValue", nullptr, nullptr), 17.42, 0.0001); - EXPECT_NEAR((float)*raw.at("floatValue", nullptr, nullptr), 66.67, 0.00001); - EXPECT_STREQ( - ((std::string)*raw.at("stringValue", nullptr, nullptr)).c_str(), - "helloworld"); - EXPECT_EQ((bool)*raw.at("boolValue", nullptr, nullptr), true); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_NEAR((double)*raw.at("doubleValue"), 17.42, 0.0001); + EXPECT_NEAR((float)*raw.at("floatValue"), 66.67, 0.00001); + EXPECT_STREQ(((std::string)*raw.at("stringValue")).c_str(), "helloworld"); + EXPECT_EQ((bool)*raw.at("boolValue"), true); + + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_NEAR((double)*raw.at("doubleValue"), 17.42, 0.0001); + EXPECT_NEAR((float)*raw.at("floatValue"), 66.67, 0.00001); + EXPECT_STREQ(((std::string)*raw.at("stringValue")).c_str(), "helloworld"); + EXPECT_EQ((bool)*raw.at("boolValue"), true); } TEST(RawPropsTest, handleRawPropsPrimitiveTypesGetOutOfOrder) { @@ -301,21 +295,17 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesGetOutOfOrder) { parser.prepare(); raw.parse(parser); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_NEAR((double)*raw.at("doubleValue", nullptr, nullptr), 17.42, 0.0001); - EXPECT_NEAR((float)*raw.at("floatValue", nullptr, nullptr), 66.67, 0.00001); - EXPECT_STREQ( - ((std::string)*raw.at("stringValue", nullptr, nullptr)).c_str(), - "helloworld"); - EXPECT_EQ((bool)*raw.at("boolValue", nullptr, nullptr), true); - - EXPECT_NEAR((double)*raw.at("doubleValue", nullptr, nullptr), 17.42, 0.0001); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_NEAR((float)*raw.at("floatValue", nullptr, nullptr), 66.67, 0.00001); - EXPECT_STREQ( - ((std::string)*raw.at("stringValue", nullptr, nullptr)).c_str(), - "helloworld"); - EXPECT_EQ((bool)*raw.at("boolValue", nullptr, nullptr), true); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_NEAR((double)*raw.at("doubleValue"), 17.42, 0.0001); + EXPECT_NEAR((float)*raw.at("floatValue"), 66.67, 0.00001); + EXPECT_STREQ(((std::string)*raw.at("stringValue")).c_str(), "helloworld"); + EXPECT_EQ((bool)*raw.at("boolValue"), true); + + EXPECT_NEAR((double)*raw.at("doubleValue"), 17.42, 0.0001); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_NEAR((float)*raw.at("floatValue"), 66.67, 0.00001); + EXPECT_STREQ(((std::string)*raw.at("stringValue")).c_str(), "helloworld"); + EXPECT_EQ((bool)*raw.at("boolValue"), true); } TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncomplete) { @@ -328,13 +318,13 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncomplete) { parser.prepare(); raw.parse(parser); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_EQ(raw.at("doubleValue", nullptr, nullptr), nullptr); - EXPECT_EQ(raw.at("floatValue", nullptr, nullptr), nullptr); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); - EXPECT_EQ(raw.at("stringValue", nullptr, nullptr), nullptr); - EXPECT_EQ(raw.at("boolValue", nullptr, nullptr), nullptr); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_EQ(raw.at("doubleValue"), nullptr); + EXPECT_EQ(raw.at("floatValue"), nullptr); + EXPECT_EQ((int)*raw.at("intValue"), 42); + EXPECT_EQ(raw.at("stringValue"), nullptr); + EXPECT_EQ(raw.at("boolValue"), nullptr); + EXPECT_EQ((int)*raw.at("intValue"), 42); } #ifdef REACT_NATIVE_DEBUG @@ -351,8 +341,8 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncorrectLookup) { // Before D18662135, looking up an invalid key would trigger // an infinite loop. This is out of contract, so we should only // test this in debug. - EXPECT_EQ(raw.at("flurb", nullptr, nullptr), nullptr); - EXPECT_EQ((int)*raw.at("intValue", nullptr, nullptr), 42); + EXPECT_EQ(raw.at("flurb"), nullptr); + EXPECT_EQ((int)*raw.at("intValue"), 42); } #endif