Skip RawPropsParser::prepare<T>() when the iterator-setter path is active (#57329)#57329
Open
javache wants to merge 3 commits into
Open
Skip RawPropsParser::prepare<T>() when the iterator-setter path is active (#57329)#57329javache wants to merge 3 commits into
RawPropsParser::prepare<T>() when the iterator-setter path is active (#57329)#57329javache wants to merge 3 commits into
Conversation
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 D109569571. |
javache
added a commit
to javache/react-native
that referenced
this pull request
Jun 25, 2026
…active (react#57329) Summary: Pull Request resolved: react#57329 `RawPropsParser::prepare<ConcreteProps>()` runs in `ConcreteComponentDescriptor`'s constructor and builds the parser's `keys_` vector + `nameToIndex_` length-bucketed map by walking every `convertRawProp` call in a probe construction of `ConcreteProps`. The cost is O(n²) in the number of props (the comment in `RawPropsParser::at` notes 4950 lookups for a 100-prop class) and is paid once per component class at app startup. Those data structures are consumed **only** by `RawProps::at()`, which is reached exclusively through the classic per-field `convertRawProp` path. The iterator-setter path skips `parse()` entirely (see the prior diff in the stack), so the prepared parser is dead weight when both: 1. `HasIteratorSetterCtor<ConcreteProps>` is satisfied (i.e. the type opts into the iterator-setter path), AND 2. `enableCppPropsIteratorSetter()` is on at runtime. Guard the call accordingly. Classes that don't satisfy the concept always run `prepare<T>()` (they can only use the classic path); when the runtime flag is off, all classes run it (since they all fall back to classic). Changelog: [Internal] Differential Revision: D109569571
meta-codesync
Bot
changed the title
RawPropsParser::prepare<T>() when the iterator-setter path is activeRawPropsParser::prepare<T>() when the iterator-setter path is active (#57329)
javache
force-pushed
the
export-D109569571
branch
from
3f22899 to
2ec5eb4
Compare
javache
added a commit
to javache/react-native
that referenced
this pull request
Jun 25, 2026
…active (react#57329) Summary: `RawPropsParser::prepare<ConcreteProps>()` runs in `ConcreteComponentDescriptor`'s constructor and builds the parser's `keys_` vector + `nameToIndex_` length-bucketed map by walking every `convertRawProp` call in a probe construction of `ConcreteProps`. The cost is O(n²) in the number of props (the comment in `RawPropsParser::at` notes 4950 lookups for a 100-prop class) and is paid once per component class at app startup. Those data structures are consumed **only** by `RawProps::at()`, which is reached exclusively through the classic per-field `convertRawProp` path. The iterator-setter path skips `parse()` entirely (see the prior diff in the stack), so the prepared parser is dead weight when both: 1. `HasIteratorSetterCtor<ConcreteProps>` is satisfied (i.e. the type opts into the iterator-setter path), AND 2. `enableCppPropsIteratorSetter()` is on at runtime. Guard the call accordingly. Classes that don't satisfy the concept always run `prepare<T>()` (they can only use the classic path); when the runtime flag is off, all classes run it (since they all fall back to classic). Changelog: [Internal] Differential Revision: D109569571
javache
force-pushed
the
export-D109569571
branch
from
2ec5eb4 to
0955e94
Compare
javache
added a commit
to javache/react-native
that referenced
this pull request
Jun 25, 2026
…active (react#57329) Summary: Pull Request resolved: react#57329 `RawPropsParser::prepare<ConcreteProps>()` runs in `ConcreteComponentDescriptor`'s constructor and builds the parser's `keys_` vector + `nameToIndex_` length-bucketed map by walking every `convertRawProp` call in a probe construction of `ConcreteProps`. The cost is O(n²) in the number of props (the comment in `RawPropsParser::at` notes 4950 lookups for a 100-prop class) and is paid once per component class at app startup. Those data structures are consumed **only** by `RawProps::at()`, which is reached exclusively through the classic per-field `convertRawProp` path. The iterator-setter path skips `parse()` entirely (see the prior diff in the stack), so the prepared parser is dead weight when both: 1. `HasIteratorSetterCtor<ConcreteProps>` is satisfied (i.e. the type opts into the iterator-setter path), AND 2. `enableCppPropsIteratorSetter()` is on at runtime. Guard the call accordingly. Classes that don't satisfy the concept always run `prepare<T>()` (they can only use the classic path); when the runtime flag is off, all classes run it (since they all fall back to classic). Changelog: [Internal] Differential Revision: D109569571
javache
force-pushed
the
export-D109569571
branch
from
0955e94 to
dd2087f
Compare
…act#57330) Summary: Auditing every Props .cpp file in the renderer surfaced six cases where a prop key parsed by the 3-arg constructor's `convertRawProp(...)` call had no matching `case` in the same class's `setProp` method. With the iterator-setter path now routing every prop through `setProp` (instead of re-running the parsing constructor), each gap silently drops the prop on the floor when the iterator-setter path is active. Fix each gap: - `BaseScrollViewProps::setProp` — add `RAW_SET_PROP_SWITCH_CASE_BASIC(automaticallyAdjustKeyboardInsets)`. Parsed in ctor, missing case. - `BaseTextProps::setProp` — add `REBUILD_FIELD_SWITCH_CASE(... dynamicTypeRamp, "dynamicTypeRamp")`. Parsed in ctor, missing case. - `BaseTextProps::setProp` — rename the `baseWritingDirection` switch case key to `writingDirection`. JS sends `writingDirection` (the C++ field is named `baseWritingDirection` internally, but JS / Flow / TS / codegen all use `writingDirection` — verified across `StyleSheetTypes.js`, `StyleSheetTypes.d.ts`, `ReactNativeStyleAttributes.js`, `RCTTextInputViewConfig.js`, and the `Text-itest.js` integration test mounts `<rn-paragraph writingDirection="rtl">`). The setProp case was keyed off the C++ field name and never fired for any real JS prop. `appendTextAttributesProps` still emits `baseWritingDirection` for the C++→JS diff path — that's a separate, out-of-scope inconsistency. - `AccessibilityProps::setProp` — rename the `accessibilityOrder` switch case key to `experimental_accessibilityOrder`. JS sends the prefixed name (verified in `ViewPropTypes.js`, `BaseViewConfig.android.js`, `BaseViewConfig.ios.js`, `ViewProps.kt`'s `ACCESSIBILITY_ORDER = "experimental_accessibilityOrder"` constant, and `HostPlatformViewProps::getDebugProps` which serializes back out as `experimental_accessibilityOrder`). The unprefixed case never fired. - `BaseViewProps::setProp` — add `RAW_SET_PROP_SWITCH_CASE_BASIC(transformOrigin)`. Parsed in ctor, missing case. - `BaseViewProps::setProp` — add `SET_CASCADED_RECTANGLE_CORNERS(borderCurves, "border", "Curve", value)`. The ctor parses the full 13-key cascaded set (`borderCurve`, `borderTopLeftCurve`, … `borderStartStartCurve`) via `CascadedRectangleCornersNames`, but `setProp` had `SET_CASCADED_RECTANGLE_CORNERS` for `borderRadii` and `SET_CASCADED_RECTANGLE_EDGES` for `borderColors`/`borderStyles` — the corresponding `borderCurves` invocation was missing. Same gaps in the `third-party/react-native-macos/.../BaseViewProps.cpp` mirror — applied the same fixes there. Out-of-scope but flagged by the audit (left for follow-ups): - `propsConversions.h`'s `ViewEvents` converter doesn't handle `onGotPointerCapture` / `onLostPointerCapture` (explicit `// TODO` at line 541). `BaseViewProps::setProp` does. Asymmetry runs the other direction — iterator-setter handles these, classic ctor doesn't. - android `HostPlatformViewProps.cpp` defines `VIEW_EVENT_CASE` macro but never invokes it in its switch — dead code (events live on `BaseViewProps::events`). Cleanup candidate. Changelog: [General][Fixed] - Several view, text, scrollview, and accessibility props that the iterator-setter path silently dropped now propagate correctly through `setProp`: `automaticallyAdjustKeyboardInsets`, `dynamicTypeRamp`, `writingDirection`, `experimental_accessibilityOrder`, `transformOrigin`, and the full `borderCurves` cascaded set. Differential Revision: D109584760
…: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
…active (react#57329) Summary: `RawPropsParser::prepare<ConcreteProps>()` runs in `ConcreteComponentDescriptor`'s constructor and builds the parser's `keys_` vector + `nameToIndex_` length-bucketed map by walking every `convertRawProp` call in a probe construction of `ConcreteProps`. The cost is O(n²) in the number of props (the comment in `RawPropsParser::at` notes 4950 lookups for a 100-prop class) and is paid once per component class at app startup. Those data structures are consumed **only** by `RawProps::at()`, which is reached exclusively through the classic per-field `convertRawProp` path. The iterator-setter path skips `parse()` entirely (see the prior diff in the stack), so the prepared parser is dead weight when both: 1. `HasIteratorSetterCtor<ConcreteProps>` is satisfied (i.e. the type opts into the iterator-setter path), AND 2. `enableCppPropsIteratorSetter()` is on at runtime. Guard the call accordingly. Classes that don't satisfy the concept always run `prepare<T>()` (they can only use the classic path); when the runtime flag is off, all classes run it (since they all fall back to classic). Changelog: [Internal] Reviewed By: christophpurrer, lenaic Differential Revision: D109569571
javache
force-pushed
the
export-D109569571
branch
from
dd2087f to
f4f2eae
Compare
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
1 participant
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:
RawPropsParser::prepare<ConcreteProps>()runs inConcreteComponentDescriptor's constructor and builds the parser'skeys_vector +nameToIndex_length-bucketed map by walking everyconvertRawPropcall in a probe construction ofConcreteProps. The cost is O(n²) in the number of props (the comment inRawPropsParser::atnotes 4950 lookups for a 100-prop class) and is paid once per component class at app startup.Those data structures are consumed only by
RawProps::at(), which is reached exclusively through the classic per-fieldconvertRawProppath. The iterator-setter path skipsparse()entirely (see the prior diff in the stack), so the prepared parser is dead weight when both:HasIteratorSetterCtor<ConcreteProps>is satisfied (i.e. the type opts into the iterator-setter path), ANDenableCppPropsIteratorSetter()is on at runtime.Guard the call accordingly. Classes that don't satisfy the concept always run
prepare<T>()(they can only use the classic path); when the runtime flag is off, all classes run it (since they all fall back to classic).Changelog:
[Internal]
Reviewed By: christophpurrer, lenaic
Differential Revision: D109569571