Skip to content

Commit 321fcde

Browse files
javachemeta-codesync[bot]
javache
authored and
meta-codesync[bot]
committed
Route enableCppPropsIteratorSetter through a copy ctor + RawProps::forEachItem
Summary: Today the iterator-setter path in `ConcreteComponentDescriptor::cloneProps` runs three sequential walks over the input — `RawProps::parse(parser)` (builds `keyIndexToValueIndex_` for `convertRawProp`), `static_cast<folly::dynamic>(rawProps)` (materializes a `folly::dynamic` via `jsi::dynamicFromValue` in JSI mode), and then `dynamic.items()` to dispatch `setProp`. Only the third is actually used: `convertRawProp` is never called on the iterator-setter branch, and the `folly::dynamic` materialization exists only as iteration scaffolding. Restructure so the runtime flag picks one of two construction paths up front: - **Iterator-setter** — copy-construct from `sourceProps` via the (re-enabled) `Props` copy ctor, then walk `rawProps` in-place via the new `RawProps::forEachItem` helper and route each entry through `setProp`. `parse()` is skipped entirely; the `folly::dynamic` materialization is skipped in `Mode::JSI`. - **Classic** — unchanged: `parse()` + 3-arg `convertRawProp`-driven ctor. `forEachItem` switches on `RawProps::Mode`: - `Mode::JSI` — walks `value_.asObject(*runtime_).getPropertyNames(...)` and constructs `RawValue` from each `jsi::Value` directly, no `folly::dynamic` in between. - `Mode::Dynamic` — iterates `dynamic_.items()` (same as today). - `Mode::Empty` — no-op. A new `HasIteratorSetterCtor<T>` concept (`std::copy_constructible<T>`) documents the contract and feeds a `static_assert` in `cloneProps`, so a future Props type that deletes its copy ctor fails at compile time rather than silently diverging at runtime between the two flag states. The `RN_SERIALIZABLE_STATE` Props 2.0 accumulation branch keeps its existing dynamic-iteration shape — when `fallbackToDynamicRawPropsAccumulation` is true, `initializeDynamicProps` has already merged the source's rawProps with the input onto `shadowNodeProps->rawProps`, so we iterate that merged dynamic rather than the raw input. The per-field `flag ? sourceProps.X : convertRawProp(...)` ternaries across every Props .cpp file become dead in the flag-on path (the copy ctor handles those fields) but are still functional in the flag-off path. They get removed in a follow-up cleanup; this diff is structurally non-breaking on either flag state. Changelog: [Internal] Differential Revision: D109568749
1 parent c9ef175 commit 321fcde

13 files changed

Lines changed: 294 additions & 23 deletions

‎packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,30 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
112112
ShadowNodeT::filterRawProps(rawProps);
113113
}
114114

115-
rawProps.parse(rawPropsParser_);
115+
// Two construction paths:
116+
// - Iterator-setter (only available when `ConcreteProps` satisfies
117+
// `HasIteratorSetterCtor` AND the runtime flag is on): copy-construct
118+
// from sourceProps, then walk rawProps in-place via `forEachItem` and
119+
// route each entry through `setProp`. Skips both
120+
// `RawProps::parse(parser)` and the `folly::dynamic` materialization
121+
// that the legacy path needed.
122+
// - Classic (the fallback for any `ConcreteProps` that doesn't opt in,
123+
// and the only path when the flag is off): parse + per-field
124+
// `convertRawProp` via the 3-arg ctor.
125+
constexpr bool kSupportsIteratorSetter = HasIteratorSetterCtor<ConcreteProps>;
126+
const bool useIteratorSetter = kSupportsIteratorSetter && ReactNativeFeatureFlags::enableCppPropsIteratorSetter();
127+
128+
std::shared_ptr<ConcreteProps> shadowNodeProps;
129+
if constexpr (kSupportsIteratorSetter) {
130+
if (useIteratorSetter) {
131+
shadowNodeProps = ShadowNodeT::Props(props);
132+
}
133+
}
134+
if (!useIteratorSetter) {
135+
rawProps.parse(rawPropsParser_);
136+
shadowNodeProps = ShadowNodeT::Props(context, rawProps, props);
137+
}
116138

117-
auto shadowNodeProps = ShadowNodeT::Props(context, rawProps, props);
118139
#ifdef RN_SERIALIZABLE_STATE
119140
bool fallbackToDynamicRawPropsAccumulation = true;
120141
if (ReactNativeFeatureFlags::enableExclusivePropsUpdateAndroid() &&
@@ -134,19 +155,23 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
134155
ShadowNodeT::initializeDynamicProps(shadowNodeProps, rawProps, props);
135156
}
136157
#endif
137-
// Use the new-style iterator
138-
// Note that we just check if `Props` has this flag set, no matter
139-
// the type of ShadowNode; it acts as the single global flag.
140-
if (ReactNativeFeatureFlags::enableCppPropsIteratorSetter()) {
158+
159+
if constexpr (kSupportsIteratorSetter) {
160+
if (useIteratorSetter) {
141161
#ifdef RN_SERIALIZABLE_STATE
142-
const auto &dynamic =
143-
fallbackToDynamicRawPropsAccumulation ? shadowNodeProps->rawProps : static_cast<folly::dynamic>(rawProps);
144-
#else
145-
const auto &dynamic = static_cast<folly::dynamic>(rawProps);
162+
if (fallbackToDynamicRawPropsAccumulation) {
163+
// Accumulation already merged the source's rawProps with the input;
164+
// iterate the merged dynamic so setProp sees the unioned set.
165+
for (const auto &pair : shadowNodeProps->rawProps.items()) {
166+
const auto &name = pair.first.getString();
167+
shadowNodeProps->setProp(context, RAW_PROPS_KEY_HASH(name), name.c_str(), RawValue(pair.second));
168+
}
169+
return shadowNodeProps;
170+
}
146171
#endif
147-
for (const auto &pair : dynamic.items()) {
148-
const auto &name = pair.first.getString();
149-
shadowNodeProps->setProp(context, RAW_PROPS_KEY_HASH(name), name.c_str(), RawValue(pair.second));
172+
rawProps.forEachItem([&](const char *name, const RawValue &value) {
173+
shadowNodeProps->setProp(context, RAW_PROPS_KEY_HASH(name), name, value);
174+
});
150175
}
151176
}
152177
return shadowNodeProps;

‎packages/react-native/ReactCommon/react/renderer/core/ConcreteShadowNode.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,32 @@ class ConcreteShadowNode : public BaseShadowNodeT {
7171
return BaseShadowNodeT::BaseTraits();
7272
}
7373

74+
/*
75+
* Classic / parse path: construct `PropsT` by parsing `rawProps` field-by-
76+
* field via the 3-arg `(context, sourceProps, rawProps)` constructor.
77+
* `ConcreteComponentDescriptor::cloneProps` calls this when the
78+
* iterator-setter path is disabled (per-class via the
79+
* `HasIteratorSetterCtor` concept, or globally via the runtime flag).
80+
*/
7481
static UnsharedConcreteProps
7582
Props(const PropsParserContext &context, const RawProps &rawProps, const Props::Shared &baseProps = nullptr)
7683
{
7784
return std::make_shared<PropsT>(
7885
context, baseProps ? static_cast<const PropsT &>(*baseProps) : *defaultSharedProps(), rawProps);
7986
}
8087

88+
/*
89+
* Iterator-setter path: copy-construct `PropsT` from `baseProps` only.
90+
* `ConcreteComponentDescriptor::cloneProps` then walks `rawProps` via
91+
* `RawProps::forEachItem` and overwrites individual fields through
92+
* `PropsT::setProp`. Available when `PropsT` satisfies
93+
* `HasIteratorSetterCtor`.
94+
*/
95+
static UnsharedConcreteProps Props(const Props::Shared &baseProps)
96+
{
97+
return std::make_shared<PropsT>(baseProps ? static_cast<const PropsT &>(*baseProps) : *defaultSharedProps());
98+
}
99+
81100
#ifdef RN_SERIALIZABLE_STATE
82101
static void initializeDynamicProps(
83102
UnsharedConcreteProps props,

‎packages/react-native/ReactCommon/react/renderer/core/Props.h‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class Props : public virtual Sealable, public virtual DebugStringConvertible {
4141
virtual ~Props() = default;
4242
#endif
4343

44-
Props(const Props &other) = delete;
44+
Props(const Props &other) = default;
4545
Props &operator=(const Props &other) = delete;
4646

4747
/**
@@ -96,4 +96,66 @@ class Props : public virtual Sealable, public virtual DebugStringConvertible {
9696
const std::function<bool(const std::string &)> &filterObjectKeys = nullptr);
9797
};
9898

99+
namespace detail {
100+
101+
/*
102+
* Extracts the class type from a pointer-to-member-function expression.
103+
* Used in unevaluated context only.
104+
*/
105+
template <typename T, typename C>
106+
auto memberFunctionClass(T C::*) -> C;
107+
108+
} // namespace detail
109+
110+
/*
111+
* Internal: `T` declares its OWN `setProp` (not just inherits one from a
112+
* base class). `&T::setProp` resolves to a pointer-to-member-function whose
113+
* class part is the level where `setProp` was actually declared — if `T`
114+
* inherits without overriding, that class is some base, not `T`.
115+
*
116+
* Distinguishing own-declaration from inherited-declaration matters because
117+
* `setProp` is non-virtual. A subclass that adds fields but forgets to
118+
* override `setProp` would silently inherit its parent's switch — and the new
119+
* fields would never be reached by the iterator-setter dispatch.
120+
*/
121+
template <typename T>
122+
concept DeclaresOwnSetProp = std::is_same_v<decltype(detail::memberFunctionClass(&T::setProp)), T>;
123+
124+
/*
125+
* Internal: `T` exposes a `setProp(ctx, hash, name, value) -> void` callable
126+
* with the canonical Props signature, declared on `T` itself.
127+
*/
128+
template <typename T>
129+
concept HasSetProp = DeclaresOwnSetProp<T> &&
130+
requires(T &t, const PropsParserContext &ctx, RawPropsPropNameHash hash, const char *name, const RawValue &value) {
131+
{ t.setProp(ctx, hash, name, value) } -> std::same_as<void>;
132+
};
133+
134+
/*
135+
* Marks a Props type as supporting the iterator-setter construction path used
136+
* by `ConcreteComponentDescriptor::cloneProps` when
137+
* `enableCppPropsIteratorSetter` is on. The contract is:
138+
*
139+
* 1. The type is copy-constructible from a source Props (so `cloneProps`
140+
* can build the new Props by copy and then overwrite individual fields).
141+
* 2. The type descends from `Props`, anchoring the `setProp` chain in the
142+
* `Props::setProp` base case.
143+
* 3. The type declares its OWN `setProp` with the canonical signature —
144+
* not inherited — so the iterator dispatch reaches every field that the
145+
* type adds beyond its base.
146+
*
147+
* `setProp` is non-virtual; subclasses chain explicitly via
148+
* `Parent::setProp(...)`. The chain integrity beyond `T` is enforced by the
149+
* compiler at each level's `setProp` body — if a subclass calls
150+
* `Parent::setProp(...)` and `Parent` does not define one, the build fails
151+
* at that call site. This concept guards the entry point (`T` itself) and
152+
* relies on those per-level calls to keep the chain whole.
153+
*
154+
* When the concept is NOT satisfied for some `ConcreteProps`,
155+
* `cloneProps` falls through to the classic per-field `convertRawProp` path
156+
* for that component regardless of the runtime flag.
157+
*/
158+
template <typename T>
159+
concept HasIteratorSetterCtor = std::copy_constructible<T> && std::derived_from<T, Props> && HasSetProp<T>;
160+
99161
} // namespace facebook::react

‎packages/react-native/ReactCommon/react/renderer/core/RawProps.h‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,45 @@ class RawProps final {
9292
*/
9393
const RawValue *at(const char *name) const noexcept;
9494

95+
/*
96+
* Iterates the underlying source object and invokes `fn(name, value)` for
97+
* each entry, in source order. Skips parsing — does NOT require a prior
98+
* `parse(parser)` call. For `Mode::JSI` this walks the JSI object in-place
99+
* (no `folly::dynamic` materialization). For `Mode::Dynamic` it walks
100+
* `dynamic_.items()`. For `Mode::Empty` it is a no-op.
101+
*
102+
* The callback signature is `void(const char *name, const RawValue &value)`.
103+
* `name` is a null-terminated C string owned by storage that lives until
104+
* the callback returns.
105+
*/
106+
template <typename Fn>
107+
void forEachItem(Fn &&fn) const
108+
{
109+
switch (mode_) {
110+
case Mode::Empty:
111+
return;
112+
case Mode::JSI: {
113+
auto object = value_.asObject(*runtime_);
114+
auto names = object.getPropertyNames(*runtime_);
115+
auto count = names.size(*runtime_);
116+
for (size_t i = 0; i < count; ++i) {
117+
auto name = names.getValueAtIndex(*runtime_, i).asString(*runtime_).utf8(*runtime_);
118+
auto propValue = object.getProperty(*runtime_, jsi::PropNameID::forUtf8(*runtime_, name));
119+
fn(name.c_str(), RawValue{*runtime_, std::move(propValue)});
120+
}
121+
return;
122+
}
123+
case Mode::Dynamic:
124+
for (const auto &pair : dynamic_.items()) {
125+
const auto &name = pair.first.getString();
126+
fn(name.c_str(), RawValue{pair.second});
127+
}
128+
return;
129+
default:
130+
return;
131+
}
132+
}
133+
95134
private:
96135
friend class RawPropsParser;
97136

‎scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,15 @@ concept facebook::react::CSSSyntaxVisitorReturn = std::is_default_constructible_
795795
template <typename T>
796796
concept facebook::react::CSSValidCompoundDataType = facebook::react::detail::is_variant_of_data_types<T>::value;
797797
template <typename T>
798+
concept facebook::react::DeclaresOwnSetProp = std::is_same_v<decltype(facebook::react::detail::memberFunctionClass(&T::setProp)), T>;
799+
template <typename T>
800+
concept facebook::react::HasIteratorSetterCtor = std::copy_constructible<T> && std::derived_from<T, facebook::react::Props> && HasSetProp<T>;
801+
template <typename T>
802+
concept facebook::react::HasSetProp = DeclaresOwnSetProp<T> &&
803+
requires(T& t, const facebook::react::PropsParserContext& ctx, facebook::react::RawPropsPropNameHash hash, const char* name, const facebook::react::RawValue& value) {
804+
{ t.setProp(ctx, hash, name, value) } -> std::same_as<void>;
805+
};
806+
template <typename T>
798807
concept facebook::react::Hashable = !std::is_same_v<T, const char*>&& (requires(T a) {
799808
{ std::hash<T>{}(a) } -> std::convertible_to<std::size_t>;
800809
});
@@ -4127,7 +4136,7 @@ class facebook::react::PreparedTextCacheKey {
41274136
class facebook::react::Props : public virtual facebook::react::Sealable, public virtual facebook::react::DebugStringConvertible {
41284137
protected void initialize(const facebook::react::PropsParserContext& context, const facebook::react::Props& sourceProps, const facebook::react::RawProps& rawProps, const std::function<bool(const std::string&)>& filterObjectKeys = nullptr);
41294138
public Props() = default;
4130-
public Props(const facebook::react::Props& other) = delete;
4139+
public Props(const facebook::react::Props& other) = default;
41314140
public Props(const facebook::react::PropsParserContext& context, const facebook::react::Props& sourceProps, const facebook::react::RawProps& rawProps, const std::function<bool(const std::string&)>& filterObjectKeys = nullptr);
41324141
public facebook::react::Props& operator=(const facebook::react::Props& other) = delete;
41334142
public folly::dynamic rawProps;
@@ -4196,6 +4205,8 @@ class facebook::react::RawProps {
41964205
public folly::dynamic toDynamic(const std::function<bool(const std::string&)>& filterObjectKeys = nullptr) const;
41974206
public operator folly::dynamic() const;
41984207
public void parse(const facebook::react::RawPropsParser& parser) noexcept;
4208+
template <typename Fn>
4209+
public void forEachItem(Fn&& fn) const;
41994210
}
42004211

42014212
enum facebook::react::RawProps::Mode {
@@ -8254,6 +8265,7 @@ class facebook::react::ConcreteShadowNode : public BaseShadowNodeT {
82548265
public static facebook::react::ComponentHandle Handle();
82558266
public static facebook::react::ComponentName Name();
82568267
public static facebook::react::ConcreteShadowNode::ConcreteStateData initialStateData(const facebook::react::Props::Shared&, const facebook::react::ShadowNodeFamily::Shared&, const facebook::react::ComponentDescriptor&);
8268+
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::Props::Shared& baseProps);
82578269
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::PropsParserContext& context, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
82588270
public static facebook::react::ShadowNodeTraits BaseTraits();
82598271
public static void initializeDynamicProps(facebook::react::ConcreteShadowNode::UnsharedConcreteProps props, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
@@ -9938,6 +9950,8 @@ template <typename CSSColor>
99389950
std::optional<facebook::react::CSSColor> facebook::react::detail::parseLegacyHslFunction(facebook::react::CSSValueParser& parser);
99399951
template <typename CSSColor>
99409952
std::optional<facebook::react::CSSColor> facebook::react::detail::parseModernHslFunction(facebook::react::CSSValueParser& parser);
9953+
template <typename T, typename C>
9954+
C facebook::react::detail::memberFunctionClass(T C::*);
99419955
template <typename... ComponentT>
99429956
constexpr std::optional<float> facebook::react::detail::normalizeComponent(const std::variant<std::monostate, ComponentT...>& component, float baseValue);
99439957

‎scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,15 @@ concept facebook::react::CSSSyntaxVisitorReturn = std::is_default_constructible_
794794
template <typename T>
795795
concept facebook::react::CSSValidCompoundDataType = facebook::react::detail::is_variant_of_data_types<T>::value;
796796
template <typename T>
797+
concept facebook::react::DeclaresOwnSetProp = std::is_same_v<decltype(facebook::react::detail::memberFunctionClass(&T::setProp)), T>;
798+
template <typename T>
799+
concept facebook::react::HasIteratorSetterCtor = std::copy_constructible<T> && std::derived_from<T, facebook::react::Props> && HasSetProp<T>;
800+
template <typename T>
801+
concept facebook::react::HasSetProp = DeclaresOwnSetProp<T> &&
802+
requires(T& t, const facebook::react::PropsParserContext& ctx, facebook::react::RawPropsPropNameHash hash, const char* name, const facebook::react::RawValue& value) {
803+
{ t.setProp(ctx, hash, name, value) } -> std::same_as<void>;
804+
};
805+
template <typename T>
797806
concept facebook::react::Hashable = !std::is_same_v<T, const char*>&& (requires(T a) {
798807
{ std::hash<T>{}(a) } -> std::convertible_to<std::size_t>;
799808
});
@@ -3981,7 +3990,7 @@ class facebook::react::PreparedTextCacheKey {
39813990
class facebook::react::Props : public virtual facebook::react::Sealable, public virtual facebook::react::DebugStringConvertible {
39823991
protected void initialize(const facebook::react::PropsParserContext& context, const facebook::react::Props& sourceProps, const facebook::react::RawProps& rawProps, const std::function<bool(const std::string&)>& filterObjectKeys = nullptr);
39833992
public Props() = default;
3984-
public Props(const facebook::react::Props& other) = delete;
3993+
public Props(const facebook::react::Props& other) = default;
39853994
public Props(const facebook::react::PropsParserContext& context, const facebook::react::Props& sourceProps, const facebook::react::RawProps& rawProps, const std::function<bool(const std::string&)>& filterObjectKeys = nullptr);
39863995
public facebook::react::Props& operator=(const facebook::react::Props& other) = delete;
39873996
public folly::dynamic rawProps;
@@ -4040,6 +4049,8 @@ class facebook::react::RawProps {
40404049
public folly::dynamic toDynamic(const std::function<bool(const std::string&)>& filterObjectKeys = nullptr) const;
40414050
public operator folly::dynamic() const;
40424051
public void parse(const facebook::react::RawPropsParser& parser) noexcept;
4052+
template <typename Fn>
4053+
public void forEachItem(Fn&& fn) const;
40434054
}
40444055

40454056
enum facebook::react::RawProps::Mode {
@@ -8018,6 +8029,7 @@ class facebook::react::ConcreteShadowNode : public BaseShadowNodeT {
80188029
public static facebook::react::ComponentHandle Handle();
80198030
public static facebook::react::ComponentName Name();
80208031
public static facebook::react::ConcreteShadowNode::ConcreteStateData initialStateData(const facebook::react::Props::Shared&, const facebook::react::ShadowNodeFamily::Shared&, const facebook::react::ComponentDescriptor&);
8032+
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::Props::Shared& baseProps);
80218033
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::PropsParserContext& context, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
80228034
public static facebook::react::ShadowNodeTraits BaseTraits();
80238035
public static void initializeDynamicProps(facebook::react::ConcreteShadowNode::UnsharedConcreteProps props, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
@@ -9564,6 +9576,8 @@ template <typename CSSColor>
95649576
std::optional<facebook::react::CSSColor> facebook::react::detail::parseLegacyHslFunction(facebook::react::CSSValueParser& parser);
95659577
template <typename CSSColor>
95669578
std::optional<facebook::react::CSSColor> facebook::react::detail::parseModernHslFunction(facebook::react::CSSValueParser& parser);
9579+
template <typename T, typename C>
9580+
C facebook::react::detail::memberFunctionClass(T C::*);
95679581
template <typename... ComponentT>
95689582
constexpr std::optional<float> facebook::react::detail::normalizeComponent(const std::variant<std::monostate, ComponentT...>& component, float baseValue);
95699583

0 commit comments

Comments
 (0)