Skip to content

Conversation

@pchalupa
Copy link
Contributor

@pchalupa pchalupa commented Jun 4, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

This follow-up on #4892 removes deprecated appOwnership from the environment utils.
This PR proposes a detection of the Expo Go app based on the presence of the ExpoGo module. This assumption is based on the following comment and testing a sample app in Expo Go and dev client apps.

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

LGTM 🚀
Thank you for following up and your contribution @pchalupa 🙇

@krystofwoldrich
Copy link
Contributor

krystofwoldrich commented Jun 5, 2025

Hi @pchalupa,
the changes make sense, and it's good to align with the Expo implementation on the Expo Go detection.


@antonis My only concern is that ExpoGo module is not available in Expo SDK 49, which is officially not supported by the Sentry RN SDK v6, but there are no know issues at the moment.

Since we are currently working on a Sentry RN SDK v7, it's up to discussion, but I think this would be a good addition to it.

@antonis
Copy link
Contributor

antonis commented Jun 5, 2025

My only concern is that ExpoGo module is not available in Expo SDK 49, which is officially not supported by the Sentry RN SDK v6, but there are no know issues at the moment.

Since we are currently working on a Sentry RN SDK v7, it's up to discussion, but I think this would be a good addition to it.

Good point @krystofwoldrich 👍 Yes, let's iterate on this in the context of v7.

@krystofwoldrich krystofwoldrich changed the base branch from main to v7 June 5, 2025 09:55
@pchalupa pchalupa requested a review from lucas-zimerman June 5, 2025 12:48
Copy link
Contributor

@krystofwoldrich krystofwoldrich left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@krystofwoldrich krystofwoldrich merged commit 480a5d3 into getsentry:v7 Jun 5, 2025
47 checks passed
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