Skip to content

fix(ios): guard null oldProps in EaseView Fabric updates#12

Open
oferRounds wants to merge 1 commit intoAppAndFlow:mainfrom
oferRounds:fix/ios-easeview-null-oldprops
Open

fix(ios): guard null oldProps in EaseView Fabric updates#12
oferRounds wants to merge 1 commit intoAppAndFlow:mainfrom
oferRounds:fix/ios-easeview-null-oldprops

Conversation

@oferRounds
Copy link

Problem

EaseView could crash with EXC_BAD_ACCESS in updateProps:oldProps: when reading fields on oldViewProps (e.g. animateTranslateX). On React Native Fabric, oldProps may be a null shared_ptr on some update paths. Dereferencing the result of static_pointer_cast without a null check is undefined behavior.

Solution

Before [super updateProps:...], resolve a non-null EaseViewProps pointer for prop diffing:

  1. Use oldProps when non-null and the cast succeeds.
  2. Otherwise use _props (still the previous generation before super updates it).
  3. Otherwise fall back to props so the diff sees no artificial changes.

Context

Observed on React Native 0.84 with the New Architecture enabled, with transform-related EaseView updates.

Testing

  • Manual: scenario that previously reproduced the crash no longer crashes.
  • Consider adding a Fabric regression test if the project has CI for iOS.

Made with Cursor

Fabric can pass a null oldProps shared_ptr; dereferencing caused EXC_BAD_ACCESS
when comparing transform props. Resolve previous EaseViewProps from oldProps,
then _props (before super), then props as fallback.

Made-with: Cursor
}
if (!previousEaseViewPropsForDiff) {
previousEaseViewPropsForDiff =
std::static_pointer_cast<const EaseViewProps>(_props);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to check both _props and props?

Could we just use props if old props is null? (remove _props check)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants