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

recording: fix crash when calling view.isVisible #201

Merged
merged 3 commits into from
Oct 25, 2024
Merged

Conversation

marandaneto
Copy link
Member

@marandaneto marandaneto commented Oct 25, 2024

💡 Motivation and Context

Relates to https://github.com/PostHog/posthog-react-native-session-replay/issues/10

💚 How did you test it?

Running method out of the main thread
It should not crash from the main thread anyway but for some reason, it does for that issue

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

return visible
}
try {
if (isAttachedToWindow) {
Copy link
Member Author

@marandaneto marandaneto Oct 25, 2024

Choose a reason for hiding this comment

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

this is vendoring the most important part of onInitializeAccessibilityNodeInfo
but we don't have control over the internal impl nor do I know if it requires the main thread or not since I cannot reproduce the original issue, so I copied and adapted and added a try-catch just in case

Copy link

@ioannisj ioannisj left a comment

Choose a reason for hiding this comment

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

Yeah, let's try this since I would assume more react-native people are experiencing this kind of crash

@marandaneto marandaneto merged commit 3ae6c6a into main Oct 25, 2024
5 checks passed
@marandaneto marandaneto deleted the fix/isVisible branch October 25, 2024 11:44
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