Skip to content

Commit c4b9224

Browse files
javachefacebook-github-bot
javache
authored and
facebook-github-bot
committed
Route enableCppPropsIteratorSetter through a copy ctor + RawProps::forEachItem (#57328)
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 fe16cf8 commit c4b9224

14 files changed

Lines changed: 286 additions & 32 deletions

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

Lines changed: 29 additions & 15 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,12 @@ 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()) {
141-
#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);
146-
#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));
158+
159+
if constexpr (kSupportsIteratorSetter) {
160+
if (useIteratorSetter) {
161+
rawProps.forEachItem([&](std::string_view name, const RawValue &value) {
162+
shadowNodeProps->setProp(context, RAW_PROPS_KEY_HASH(name), name.data(), value);
163+
});
150164
}
151165
}
152166
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.cpp‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,7 @@ Props::Props(
2929
rawProps,
3030
"nativeID",
3131
sourceProps.nativeId,
32-
{})) {
33-
#ifdef RN_SERIALIZABLE_STATE
34-
if (!ReactNativeFeatureFlags::enableExclusivePropsUpdateAndroid()) {
35-
initializeDynamicProps(sourceProps, rawProps, filterObjectKeys);
36-
}
37-
#endif
38-
}
32+
{})) {}
3933

4034
void Props::setProp(
4135
const PropsParserContext& context,

‎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
/**
@@ -84,4 +84,66 @@ class Props : public virtual Sealable, public virtual DebugStringConvertible {
8484
#endif
8585
};
8686

87+
namespace detail {
88+
89+
/*
90+
* Extracts the class type from a pointer-to-member-function expression.
91+
* Used in unevaluated context only.
92+
*/
93+
template <typename T, typename C>
94+
auto memberFunctionClass(T C::*) -> C;
95+
96+
} // namespace detail
97+
98+
/*
99+
* Internal: `T` declares its OWN `setProp` (not just inherits one from a
100+
* base class). `&T::setProp` resolves to a pointer-to-member-function whose
101+
* class part is the level where `setProp` was actually declared — if `T`
102+
* inherits without overriding, that class is some base, not `T`.
103+
*
104+
* Distinguishing own-declaration from inherited-declaration matters because
105+
* `setProp` is non-virtual. A subclass that adds fields but forgets to
106+
* override `setProp` would silently inherit its parent's switch — and the new
107+
* fields would never be reached by the iterator-setter dispatch.
108+
*/
109+
template <typename T>
110+
concept DeclaresOwnSetProp = std::is_same_v<decltype(detail::memberFunctionClass(&T::setProp)), T>;
111+
112+
/*
113+
* Internal: `T` exposes a `setProp(ctx, hash, name, value) -> void` callable
114+
* with the canonical Props signature, declared on `T` itself.
115+
*/
116+
template <typename T>
117+
concept HasSetProp = DeclaresOwnSetProp<T> &&
118+
requires(T &t, const PropsParserContext &ctx, RawPropsPropNameHash hash, const char *name, const RawValue &value) {
119+
{ t.setProp(ctx, hash, name, value) } -> std::same_as<void>;
120+
};
121+
122+
/*
123+
* Marks a Props type as supporting the iterator-setter construction path used
124+
* by `ConcreteComponentDescriptor::cloneProps` when
125+
* `enableCppPropsIteratorSetter` is on. The contract is:
126+
*
127+
* 1. The type is copy-constructible from a source Props (so `cloneProps`
128+
* can build the new Props by copy and then overwrite individual fields).
129+
* 2. The type descends from `Props`, anchoring the `setProp` chain in the
130+
* `Props::setProp` base case.
131+
* 3. The type declares its OWN `setProp` with the canonical signature —
132+
* not inherited — so the iterator dispatch reaches every field that the
133+
* type adds beyond its base.
134+
*
135+
* `setProp` is non-virtual; subclasses chain explicitly via
136+
* `Parent::setProp(...)`. The chain integrity beyond `T` is enforced by the
137+
* compiler at each level's `setProp` body — if a subclass calls
138+
* `Parent::setProp(...)` and `Parent` does not define one, the build fails
139+
* at that call site. This concept guards the entry point (`T` itself) and
140+
* relies on those per-level calls to keep the chain whole.
141+
*
142+
* When the concept is NOT satisfied for some `ConcreteProps`,
143+
* `cloneProps` falls through to the classic per-field `convertRawProp` path
144+
* for that component regardless of the runtime flag.
145+
*/
146+
template <typename T>
147+
concept HasIteratorSetterCtor = std::copy_constructible<T> && std::derived_from<T, Props> && HasSetProp<T>;
148+
87149
} // 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(std::string_view name, const RawValue &value)`.
103+
* The view points into storage owned by `forEachItem` for the duration of
104+
* the call and is null-terminated (i.e. `name.data()` is a valid C string).
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_);
118+
auto propValue = object.getProperty(*runtime_, name);
119+
auto nameUtf8 = name.utf8(*runtime_);
120+
fn(std::string_view{nameUtf8}, RawValue{*runtime_, std::move(propValue)});
121+
}
122+
return;
123+
}
124+
case Mode::Dynamic:
125+
for (const auto &pair : dynamic_.items()) {
126+
fn(std::string_view{pair.first.getString()}, 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
});
@@ -4126,7 +4135,7 @@ class facebook::react::PreparedTextCacheKey {
41264135

41274136
class facebook::react::Props : public virtual facebook::react::Sealable, public virtual facebook::react::DebugStringConvertible {
41284137
public Props() = default;
4129-
public Props(const facebook::react::Props& other) = delete;
4138+
public Props(const facebook::react::Props& other) = default;
41304139
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);
41314140
public facebook::react::Props& operator=(const facebook::react::Props& other) = delete;
41324141
public folly::dynamic rawProps;
@@ -4195,6 +4204,8 @@ class facebook::react::RawProps {
41954204
public folly::dynamic toDynamic(const std::function<bool(const std::string&)>& filterObjectKeys = nullptr) const;
41964205
public operator folly::dynamic() const;
41974206
public void parse(const facebook::react::RawPropsParser& parser) noexcept;
4207+
template <typename Fn>
4208+
public void forEachItem(Fn fn) const;
41984209
}
41994210

42004211
enum facebook::react::RawProps::Mode {
@@ -8253,6 +8264,7 @@ class facebook::react::ConcreteShadowNode : public BaseShadowNodeT {
82538264
public static facebook::react::ComponentHandle Handle();
82548265
public static facebook::react::ComponentName Name();
82558266
public static facebook::react::ConcreteShadowNode::ConcreteStateData initialStateData(const facebook::react::Props::Shared&, const facebook::react::ShadowNodeFamily::Shared&, const facebook::react::ComponentDescriptor&);
8267+
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::Props::Shared& baseProps);
82568268
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::PropsParserContext& context, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
82578269
public static facebook::react::ShadowNodeTraits BaseTraits();
82588270
public static void initializeDynamicProps(facebook::react::ConcreteShadowNode::UnsharedConcreteProps props, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
@@ -9937,6 +9949,8 @@ template <typename CSSColor>
99379949
std::optional<facebook::react::CSSColor> facebook::react::detail::parseLegacyHslFunction(facebook::react::CSSValueParser& parser);
99389950
template <typename CSSColor>
99399951
std::optional<facebook::react::CSSColor> facebook::react::detail::parseModernHslFunction(facebook::react::CSSValueParser& parser);
9952+
template <typename T, typename C>
9953+
C facebook::react::detail::memberFunctionClass(T C::*);
99409954
template <typename... ComponentT>
99419955
constexpr std::optional<float> facebook::react::detail::normalizeComponent(const std::variant<std::monostate, ComponentT...>& component, float baseValue);
99429956

‎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
});
@@ -3980,7 +3989,7 @@ class facebook::react::PreparedTextCacheKey {
39803989

39813990
class facebook::react::Props : public virtual facebook::react::Sealable, public virtual facebook::react::DebugStringConvertible {
39823991
public Props() = default;
3983-
public Props(const facebook::react::Props& other) = delete;
3992+
public Props(const facebook::react::Props& other) = default;
39843993
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);
39853994
public facebook::react::Props& operator=(const facebook::react::Props& other) = delete;
39863995
public folly::dynamic rawProps;
@@ -4039,6 +4048,8 @@ class facebook::react::RawProps {
40394048
public folly::dynamic toDynamic(const std::function<bool(const std::string&)>& filterObjectKeys = nullptr) const;
40404049
public operator folly::dynamic() const;
40414050
public void parse(const facebook::react::RawPropsParser& parser) noexcept;
4051+
template <typename Fn>
4052+
public void forEachItem(Fn fn) const;
40424053
}
40434054

40444055
enum facebook::react::RawProps::Mode {
@@ -8017,6 +8028,7 @@ class facebook::react::ConcreteShadowNode : public BaseShadowNodeT {
80178028
public static facebook::react::ComponentHandle Handle();
80188029
public static facebook::react::ComponentName Name();
80198030
public static facebook::react::ConcreteShadowNode::ConcreteStateData initialStateData(const facebook::react::Props::Shared&, const facebook::react::ShadowNodeFamily::Shared&, const facebook::react::ComponentDescriptor&);
8031+
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::Props::Shared& baseProps);
80208032
public static facebook::react::ConcreteShadowNode::UnsharedConcreteProps Props(const facebook::react::PropsParserContext& context, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
80218033
public static facebook::react::ShadowNodeTraits BaseTraits();
80228034
public static void initializeDynamicProps(facebook::react::ConcreteShadowNode::UnsharedConcreteProps props, const facebook::react::RawProps& rawProps, const facebook::react::Props::Shared& baseProps = nullptr);
@@ -9563,6 +9575,8 @@ template <typename CSSColor>
95639575
std::optional<facebook::react::CSSColor> facebook::react::detail::parseLegacyHslFunction(facebook::react::CSSValueParser& parser);
95649576
template <typename CSSColor>
95659577
std::optional<facebook::react::CSSColor> facebook::react::detail::parseModernHslFunction(facebook::react::CSSValueParser& parser);
9578+
template <typename T, typename C>
9579+
C facebook::react::detail::memberFunctionClass(T C::*);
95669580
template <typename... ComponentT>
95679581
constexpr std::optional<float> facebook::react::detail::normalizeComponent(const std::variant<std::monostate, ComponentT...>& component, float baseValue);
95689582

0 commit comments

Comments
 (0)