Skip to content

Fix ctor/setProp prop-name gaps surfaced by iterator-setter audit#57330

Open
javache wants to merge 2 commits into
react:mainfrom
javache:export-D109584760
Open

Fix ctor/setProp prop-name gaps surfaced by iterator-setter audit#57330
javache wants to merge 2 commits into
react:mainfrom
javache:export-D109584760

Conversation

@javache

@javache javache commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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

NickGerleman and others added 2 commits June 24, 2026 10:19
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
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
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 25, 2026
@meta-codesync

meta-codesync Bot commented Jun 25, 2026

Copy link
Copy Markdown

@javache has exported this pull request. If you are a Meta employee, you can view the originating Diff in D109584760.

javache added a commit to javache/react-native that referenced this pull request Jun 25, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants