Skip to content

Reset removed shadow, editable and persistentScrollbar props to their defaults - #59125

Open
YevheniiKotyrlo wants to merge 3 commits into
react:mainfrom
YevheniiKotyrlo:fix/removed-prop-defaults
Open

YevheniiKotyrlo wants to merge 3 commits into
react:mainfrom
YevheniiKotyrlo:fix/removed-prop-defaults

Conversation

@YevheniiKotyrlo

@YevheniiKotyrlo YevheniiKotyrlo commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary:

A prop removed from a component reaches the C++ props as null, and convertRawProp then returns its fallback argument. For these props the fallback differs from the field's declared default, so a component that had the prop and lost it is left with a value that a component which never had it does not have:

Prop Declared default After removal
shadowRadius 3 0
shadowOffset {width: 0, height: -3} {width: 0, height: 0}
editable (TextInput) true false
persistentScrollbar (ScrollView) false true
sendMomentumEvents, nestedScrollEnabled (Android ScrollView) false true

On iOS, RCTViewComponentView applies the shadow values to the layer, so a view that sets shadowRadius or shadowOffset conditionally (a raised card, a pressed state) gets layer.shadowRadius = 0 and layer.shadowOffset = {0, 0} once they are removed, instead of the defaults it started with. On Android, the props diff of enablePropsUpdateReconciliationAndroid reads the same values, so a TextInput that drops editable={false} stays read-only and a ScrollView that drops persistentScrollbar keeps it. ScrollView always passes sendMomentumEvents and nestedScrollEnabled, so those two only reach a direct user of the native component.

Each fallback is now the declared default, the value setProp (enableCppPropsIteratorSetter) already resets these props to.

Changelog:

[GENERAL] [FIXED] - Reset a removed shadowRadius, shadowOffset, editable or persistentScrollbar prop to its default

Test Plan:

  • ViewPropsTest and BaseTextInputPropsTest parse a prop, then its removal, and compare the result with a default-constructed props object. On main they fail with removed.shadowRadius 0 against 3, removed.shadowOffset.height 0 against -3, and removed.editable false against true. BaseTextInputProps had no tests, so textinput/tests/ is new: the SwiftPM target excludes it as the other targets exclude theirs, and it needs a test target in the internal build.
  • A Fantom case in ScrollView-itest.js renders persistentScrollbar={true} and then no prop; on main the second render still carries persistentScrollbar="true".
  • I ran these in the reactnativecommunity/react-native-android image, linking the gtests against the Fantom runner's ReactCommon build: the new tests fail on main and pass with this change, and the whole Fantom suite passes with it. clang-format 21.1.2, Prettier, ESLint and Flow pass.
  • RNTester release builds of main and of this change on an iPad (iOS 27.0.1): a view that loses shadowRadius and shadowOffset draws no shadow on main and the default one with this change (before and after).

A Fantom case removes persistentScrollbar from a ScrollView, and gtests
parse a removed shadowRadius, shadowOffset and editable, each expected
to equal the value of a component that never set the prop.
… defaults

A removed prop reaches the C++ props as null, and convertRawProp returns
its fallback argument for it. shadowRadius, shadowOffset, editable,
persistentScrollbar, sendMomentumEvents and nestedScrollEnabled passed a
fallback other than their declared default, so a component that dropped
the prop kept a value that a component which never set it does not have.
Each fallback is now the declared default, which setProp already uses.
@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 Oct 16, 2026
The target compiles every source below its path, so the gtest in
textinput/tests needs the same exclusion the other targets give theirs.
@YevheniiKotyrlo

Copy link
Copy Markdown
Contributor Author

Device evidence — before / after

UNFIXED — the right card loses its shadow once shadowRadius and shadowOffset are removed: its layer is left at radius 0 and offset {0, 0}, so the shadow sits behind the card.

FIXED — the right card draws the default shadow, like the left card, which never had either prop.

before — main at 68b7ce2abe after — with this PR

iPad Pro 12.9-inch (4th generation), iOS 27.0.1: RNTester release builds of main at 68b7ce2abe and of this PR at 4968cd54b7, one session, with the scene below as the root component and a label naming the build. Both cards keep shadowColor and shadowOpacity; the right one renders with shadowRadius: 12 and shadowOffset: {width: 0, height: 12}, and an effect removes both after the first commit. The two frames differ only in the right card's shadow and the label: the 8 px band above that card's top edge averages a luminance of 226.7 on main, the background's, and 105.8 with this change, the left card's.

Scene
const SHADOW = {shadowColor: '#000000', shadowOpacity: 0.9};
const SHADOW_GEOMETRY = {shadowRadius: 12, shadowOffset: {width: 0, height: 12}};

function ShadowRemovalExample() {
  const [removed, setRemoved] = React.useState(false);
  React.useEffect(() => {
    setRemoved(true);
  }, []);
  return (
    <View style={styles.row}>
      <View style={[styles.card, SHADOW]} />
      <View style={[styles.card, SHADOW, removed ? null : SHADOW_GEOMETRY]} />
    </View>
  );
}

const styles = StyleSheet.create({
  row: {flexDirection: 'row', gap: 72},
  card: {width: 200, height: 140, borderRadius: 16, backgroundColor: '#ffffff'},
});

@YevheniiKotyrlo

Copy link
Copy Markdown
Contributor Author

The red iOS jobs come from one download: test_ios_rntester (Release, false) lost its connection fetching the Hermes release tarball (curl: (56) Recv failure: Connection reset by peer), and the test_e2e_ios_rntester Release jobs then find no RNTesterApp-NewArch-Release artifact.

@YevheniiKotyrlo
YevheniiKotyrlo marked this pull request as ready for review October 16, 2026 08:08
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 16, 2026
@meta-codesync

meta-codesync Bot commented Oct 16, 2026

Copy link
Copy Markdown

@javache has imported this pull request. If you are a Meta employee, you can view this in D125706127.

This branch has not been deployed

No deployments
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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant