Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix nil safety in RiveReactNativeView.swift #206

Merged

Conversation

ckknight
Copy link
Contributor

I was able to consistently crash my React Native app when Hot Module Reloading was triggered. Removing the implicit optional unwrapping and making that explicit within RiveReactNativeView fixed the crashes I was running into.

I was able to consistently crash my React Native app when Hot Module
Reloading was triggered. Removing the implicit optional unwrapping and
making that explicit within RiveReactNativeView fixed the crashes I was
running into.
@HayesGordon
Copy link
Contributor

Hi @ckknight thank you for the contribution! I'll get this reviewed soon

@HayesGordon
Copy link
Contributor

Hi @ckknight, could you share a small repro that demonstrates the crash. I've not been able to reproduce the issue my side. Thank you in advance for the time spent on this!

@LewisYearsley
Copy link
Contributor

@HayesGordon Currently seeing this when calling play/pause imperative methods on a react ref passed to a rive instance from an effect. The problem is more reliable if you use a layout effect, which makes sense given it's synchronous.

@Waltari10
Copy link

@ckknight is it the same crash as this one? #218

rive_react_native/RiveReactNativeView.swift:225: Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value

Copy link
Contributor

@HayesGordon HayesGordon left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for the fix

@HayesGordon HayesGordon merged commit a1e1483 into rive-app:main Jan 2, 2024
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.

4 participants