Route enableCppPropsIteratorSetter through a copy ctor + RawProps::forEachItem#57328
Open
javache wants to merge 2 commits into
Open
Route enableCppPropsIteratorSetter through a copy ctor + RawProps::forEachItem#57328javache wants to merge 2 commits into
enableCppPropsIteratorSetter through a copy ctor + RawProps::forEachItem#57328javache wants to merge 2 commits into
Conversation
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 Differential Revision: D94367880 Reviewed By: javache
…: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
meta-cla
Bot
added
the
CLA Signed
|
@javache has exported this pull request. If you are a Meta employee, you can view the originating Diff in D109568749. |
javache
added a commit
to javache/react-native
that referenced
this pull request
Jun 25, 2026
…:forEachItem` (react#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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Today the iterator-setter path in
ConcreteComponentDescriptor::clonePropsruns three sequential walks over the input —RawProps::parse(parser)(buildskeyIndexToValueIndex_forconvertRawProp),static_cast<folly::dynamic>(rawProps)(materializes afolly::dynamicviajsi::dynamicFromValuein JSI mode), and thendynamic.items()to dispatchsetProp. Only the third is actually used:convertRawPropis never called on the iterator-setter branch, and thefolly::dynamicmaterialization exists only as iteration scaffolding.Restructure so the runtime flag picks one of two construction paths up front:
sourcePropsvia the (re-enabled)Propscopy ctor, then walkrawPropsin-place via the newRawProps::forEachItemhelper and route each entry throughsetProp.parse()is skipped entirely; thefolly::dynamicmaterialization is skipped inMode::JSI.parse()+ 3-argconvertRawProp-driven ctor.forEachItemswitches onRawProps::Mode:Mode::JSI— walksvalue_.asObject(*runtime_).getPropertyNames(...)and constructsRawValuefrom eachjsi::Valuedirectly, nofolly::dynamicin between.Mode::Dynamic— iteratesdynamic_.items()(same as today).Mode::Empty— no-op.A new
HasIteratorSetterCtor<T>concept (std::copy_constructible<T>) documents the contract and feeds astatic_assertincloneProps, 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_STATEProps 2.0 accumulation branch keeps its existing dynamic-iteration shape — whenfallbackToDynamicRawPropsAccumulationis true,initializeDynamicPropshas already merged the source's rawProps with the input ontoshadowNodeProps->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