-
-
Notifications
You must be signed in to change notification settings - Fork 372
feat: Add isiOSAppOnVisionOS, isiOSAppOnMac, isMacCatalystApp to device context #6939
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
base: main
Are you sure you want to change the base?
Conversation
|
…r-platforms-detection
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6939 +/- ##
=============================================
- Coverage 84.773% 84.767% -0.006%
=============================================
Files 459 459
Lines 27517 27540 +23
Branches 12130 12145 +15
=============================================
+ Hits 23327 23345 +18
- Misses 4149 4154 +5
Partials 41 41
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
@philprime are you waiting on something in particular for this? |
…r-platforms-detection
…r-platforms-detection
| } | ||
| // Fallback for older versions: `UIWindowSceneGeometryPreferencesVision` is only available on visionOS | ||
| // https://developer.apple.com/documentation/uikit/uiwindowscene/geometrypreferences/vision?language=objc | ||
| return NSClassFromString("UIWindowSceneGeometryPreferencesVision") != nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback incorrectly returns true for native visionOS apps
The fallback detection for isiOSAppOnVisionOS uses NSClassFromString("UIWindowSceneGeometryPreferencesVision") to check if running on visionOS. This class exists on all visionOS builds, so native visionOS apps on older visionOS versions (< 26.1, before the official API exists) will incorrectly return true when they should return false. Since the SDK officially supports visionOS as a native platform (per Package.swift), this could result in incorrect telemetry. A compile-time check like #if os(visionOS) could be used to always return false for native visionOS builds.
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| bce9765 | 1229.42 ms | 1243.49 ms | 14.07 ms |
| daeeb27 | 1235.35 ms | 1253.77 ms | 18.42 ms |
| 1fe932f | 1231.92 ms | 1253.44 ms | 21.52 ms |
| d23a1b1 | 1218.94 ms | 1239.53 ms | 20.60 ms |
| 5196f0d | 1213.35 ms | 1231.37 ms | 18.02 ms |
| 0a17f4a | 1206.36 ms | 1236.68 ms | 30.33 ms |
| 649265b | 1235.16 ms | 1264.59 ms | 29.43 ms |
| 5840d2d | 1225.40 ms | 1241.47 ms | 16.07 ms |
| d1c0538 | 1227.49 ms | 1246.96 ms | 19.47 ms |
| d1c4916 | 1236.25 ms | 1266.76 ms | 30.51 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| bce9765 | 23.74 KiB | 874.06 KiB | 850.32 KiB |
| daeeb27 | 23.75 KiB | 989.12 KiB | 965.37 KiB |
| 1fe932f | 23.75 KiB | 913.63 KiB | 889.88 KiB |
| d23a1b1 | 23.75 KiB | 913.64 KiB | 889.88 KiB |
| 5196f0d | 23.75 KiB | 876.93 KiB | 853.19 KiB |
| 0a17f4a | 24.14 KiB | 1.01 MiB | 1012.90 KiB |
| 649265b | 23.75 KiB | 980.81 KiB | 957.06 KiB |
| 5840d2d | 23.75 KiB | 969.24 KiB | 945.50 KiB |
| d1c0538 | 23.75 KiB | 928.87 KiB | 905.12 KiB |
| d1c4916 | 23.75 KiB | 981.15 KiB | 957.40 KiB |
📜 Description
isiOSAppOnVisionOSsupporting all visionOS versions💡 Motivation and Context
Closes #3824
Closes #3825
💚 How did you test it?
visionOS
iOS-Swiftios_app_on_visionos,ios_app_on_macosandmac_catalyst_appare setios_app_on_visionosios_app_on_macosmac_catalyst_appNote: During testing I encountered an
unrecognized selectorSDK crash on visionOS 1.1, even though it was gated with an availability check. I fixed it by adding the recognized selector check.Unit Tests
In addition I added a unit test which asserts that the flag
isiOSAppOnVisionOSis defaultfalse.📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.