Skip to content

Commit c1d8ae9

Browse files
javachemeta-codesync[bot]
authored andcommitted
Route enableCppPropsIteratorSetter through a copy ctor + RawProps::forEachItem (#57328)
Summary: Pull Request resolved: #57328 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] Reviewed By: zeyap Differential Revision: D109568749 fbshipit-source-id: eae20478418a7dbf7364c73d85d7694d99f1e8f1
1 parent 2e36e63 commit c1d8ae9

18 files changed

Lines changed: 443 additions & 42 deletions

packages/react-native/Libraries/Components/View/__tests__/View-itest.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*
77
* @flow strict-local
88
* @fantom_flags enableNativeCSSParsing:*
9+
* @fantom_flags enableCppPropsIteratorSetter:*
910
* @format
1011
*/
1112

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
@@ -97,6 +97,45 @@ class RawProps final {
9797
// compatibility with callers that pass prefix/suffix separately.
9898
const RawValue *at(const char *name, const char *prefix, const char *suffix) const noexcept;
9999

100+
/*
101+
* Iterates the underlying source object and invokes `fn(name, value)` for
102+
* each entry, in source order. Skips parsing — does NOT require a prior
103+
* `parse(parser)` call. For `Mode::JSI` this walks the JSI object in-place
104+
* (no `folly::dynamic` materialization). For `Mode::Dynamic` it walks
105+
* `dynamic_.items()`. For `Mode::Empty` it is a no-op.
106+
*
107+
* The callback signature is `void(std::string_view name, const RawValue &value)`.
108+
* The view points into storage owned by `forEachItem` for the duration of
109+
* the call and is null-terminated (i.e. `name.data()` is a valid C string).
110+
*/
111+
template <typename Fn>
112+
void forEachItem(Fn fn) const
113+
{
114+
switch (mode_) {
115+
case Mode::Empty:
116+
return;
117+
case Mode::JSI: {
118+
auto object = value_.asObject(*runtime_);
119+
auto names = object.getPropertyNames(*runtime_);
120+
auto count = names.size(*runtime_);
121+
for (size_t i = 0; i < count; ++i) {
122+
auto name = names.getValueAtIndex(*runtime_, i).getString(*runtime_);
123+
auto propValue = object.getProperty(*runtime_, name);
124+
auto nameUtf8 = name.utf8(*runtime_);
125+
fn(std::string_view{nameUtf8}, RawValue{*runtime_, std::move(propValue)});
126+
}
127+
return;
128+
}
129+
case Mode::Dynamic:
130+
for (const auto &pair : dynamic_.items()) {
131+
fn(std::string_view{pair.first.getString()}, RawValue{pair.second});
132+
}
133+
return;
134+
default:
135+
return;
136+
}
137+
}
138+
100139
private:
101140
friend class RawPropsParser;
102141

packages/react-native/ReactCommon/react/renderer/core/tests/ComponentDescriptorTest.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,20 @@
77

88
#include <gtest/gtest.h>
99

10+
#include <react/renderer/core/Props.h>
1011
#include <react/renderer/core/PropsParserContext.h>
1112

1213
#include "TestComponent.h"
1314

1415
using namespace facebook::react;
1516

17+
static_assert(
18+
HasIteratorSetterCtor<Props>,
19+
"base `Props` must itself satisfy `HasIteratorSetterCtor` — it is copy-constructible, "
20+
"derived from itself, and declares its own `setProp`. If this fails, "
21+
"`ConcreteComponentDescriptor::cloneProps` will silently fall back to the classic "
22+
"path for any concrete props whose nearest setProp-declaring ancestor is `Props` itself.");
23+
1624
TEST(ComponentDescriptorTest, createShadowNode) {
1725
auto eventDispatcher = std::shared_ptr<const EventDispatcher>();
1826
SharedComponentDescriptor descriptor =
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include <gtest/gtest.h>
9+
#include <react/renderer/core/Props.h>
10+
#include <react/renderer/core/PropsParserContext.h>
11+
#include <react/renderer/core/RawProps.h>
12+
#include <react/renderer/core/propsConversions.h>
13+
14+
using namespace facebook::react;
15+
16+
namespace {
17+
18+
// Mimics the shape the React Native codegen emits for
19+
// `codegenNativeComponent<NativeProps>(...)` (see
20+
// `react-native-codegen/src/generators/components/GeneratePropsH.js`
21+
// `ClassTemplate`): default ctor, the 3-arg classic ctor with
22+
// `convertRawProp` in its initializer list, public prop fields, and
23+
// NO `setProp` override.
24+
class CodegenStyleProps : public Props {
25+
public:
26+
CodegenStyleProps() = default;
27+
CodegenStyleProps(
28+
const PropsParserContext& context,
29+
const CodegenStyleProps& sourceProps,
30+
const RawProps& rawProps)
31+
: Props(context, sourceProps, rawProps),
32+
customField(convertRawProp(
33+
context,
34+
rawProps,
35+
"customField",
36+
sourceProps.customField,
37+
0)) {}
38+
39+
int customField{0};
40+
};
41+
42+
// Mimics a hand-written Props subclass that opts in to the
43+
// iterator-setter path by declaring its own `setProp`.
44+
class HandWrittenSetPropProps : public Props {
45+
public:
46+
HandWrittenSetPropProps() = default;
47+
HandWrittenSetPropProps(
48+
const PropsParserContext& context,
49+
const HandWrittenSetPropProps& sourceProps,
50+
const RawProps& rawProps)
51+
: Props(context, sourceProps, rawProps),
52+
customField(sourceProps.customField) {}
53+
54+
void setProp(
55+
const PropsParserContext& context,
56+
RawPropsPropNameHash hash,
57+
const char* propName,
58+
const RawValue& value) {
59+
Props::setProp(context, hash, propName, value);
60+
switch (hash) {
61+
case CONSTEXPR_RAW_PROPS_KEY_HASH("customField"):
62+
fromRawValue(context, value, customField, 0);
63+
return;
64+
}
65+
}
66+
67+
int customField{0};
68+
};
69+
70+
// A subclass of a hand-written setProp class that does NOT redeclare
71+
// `setProp`. Acts like a downstream specialization (e.g. how
72+
// `ViewShadowNodeProps` extends `ViewProps`). The concept must reject
73+
// this — otherwise the iterator-setter dispatch would silently miss
74+
// `extraField`.
75+
class InheritedSetPropProps : public HandWrittenSetPropProps {
76+
public:
77+
InheritedSetPropProps() = default;
78+
InheritedSetPropProps(
79+
const PropsParserContext& context,
80+
const InheritedSetPropProps& sourceProps,
81+
const RawProps& rawProps)
82+
: HandWrittenSetPropProps(context, sourceProps, rawProps),
83+
extraField(sourceProps.extraField) {}
84+
85+
int extraField{0};
86+
};
87+
88+
// `using Base::setProp;` is sugar — it doesn't constitute "own"
89+
// declaration. Concept must still reject.
90+
class UsingDeclSetPropProps : public HandWrittenSetPropProps {
91+
public:
92+
using HandWrittenSetPropProps::setProp;
93+
};
94+
95+
// Concept axioms — fail the build if the iterator-setter dispatch ever
96+
// regresses to silently accepting classes that inherit `setProp`.
97+
static_assert(
98+
!DeclaresOwnSetProp<CodegenStyleProps>,
99+
"Codegen'd Props classes don't declare setProp — concept must reject them.");
100+
static_assert(
101+
!HasSetProp<CodegenStyleProps>,
102+
"HasSetProp requires DeclaresOwnSetProp.");
103+
static_assert(
104+
!HasIteratorSetterCtor<CodegenStyleProps>,
105+
"Codegen'd Props must fall through to the classic cloneProps path. "
106+
"If this fires, the iterator-setter would skip every field the codegen'd "
107+
"ctor populates via convertRawProp, leaving the component unrendered.");
108+
109+
static_assert(DeclaresOwnSetProp<HandWrittenSetPropProps>);
110+
static_assert(HasSetProp<HandWrittenSetPropProps>);
111+
static_assert(HasIteratorSetterCtor<HandWrittenSetPropProps>);
112+
113+
static_assert(
114+
!DeclaresOwnSetProp<InheritedSetPropProps>,
115+
"A subclass that inherits setProp without redeclaring it must not "
116+
"satisfy the concept — its new fields would be skipped.");
117+
static_assert(!HasIteratorSetterCtor<InheritedSetPropProps>);
118+
119+
static_assert(
120+
!DeclaresOwnSetProp<UsingDeclSetPropProps>,
121+
"`using Base::setProp;` is not an own declaration.");
122+
static_assert(!HasIteratorSetterCtor<UsingDeclSetPropProps>);
123+
124+
// Baselines: `Props` itself owns `setProp`, so it satisfies the concept.
125+
static_assert(DeclaresOwnSetProp<Props>);
126+
static_assert(HasIteratorSetterCtor<Props>);
127+
128+
} // namespace
129+
130+
// gtest entry so the file participates in the test binary (the
131+
// static_asserts above are the real check — this just keeps gtest's
132+
// test discovery happy).
133+
TEST(PropsConceptsTest, ConceptAxiomsCompile) {
134+
SUCCEED();
135+
}

0 commit comments

Comments
 (0)