-
Notifications
You must be signed in to change notification settings - Fork 1k
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(iOS): Making permissions switch statements exhaustive & supporting new iOS 14 cases #3160
Conversation
…and new iOS 14 enum cases.
#if swift(>=5.3) | ||
case .limited: | ||
// TODO: address this new case properly | ||
#warning(".limited != .authorized, authorization status should be revisted for iOS 14") |
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.
In addition to the new case, authorizationStatus()
is being deprecated in favor of authorizationStatus(for:)
. The implications of .limited
aren't fully clear to me yet so I'm adding this warning to be sure that it gets revisited before the beta period is over since it might require changes across more of Capacitor. But this addresses the immediate problem of compiles breaking with Xcode 12 until that time.
Co-authored-by: Dan Imhoff <dwieeb@gmail.com>
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.
I think the @unknown default
cases should call reject with an error. In that case, if a new value comes out and we don't see it on time, when users get the error they will most likely report an issue about it that if they get "prompt" result.
ret = "denied" | ||
case .authorized: | ||
ret = "granted" | ||
#if swift(>=5.3) |
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.
The new enum cases are not bound to Swift 5.3. They are added in iOS 14 and the availability check should check for os version instead.
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.
As far as I know, they are only available on iOS 14 if you use SDK 14 to compile (Xcode 12), if you use SDK 13 (Xcode 11) then they won't be available because it works in a sort of "compatibility mode".
The #if swift(>=5.3)
is only true on Xcode 12, it's false in Xcode 11 because it ships with swift 5.1 to 5.2.4.
I agree that using if #available(iOS 14, *)
would be more clear, but sadly, it can't be used inside the switch.
Maybe is less confusing if we use #if compiler(>=5.3)
instead of #if swift(>=5.3)
?
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.
Yes, it is true that language version is not synonymous with the SDK version but in practice it's not an issue. The only way it would be a problem is if you downloaded a newer version of the Swift compiler and replaced the toolchain in Xcode 11. If you did that, you should know what you're doing and can expect side effects in many places.
Not only is #available
invalid inside a switch statement (so we'd need to have duplicated switches to use it), it doesn't actually solve our problem because that's a hook to insert a runtime check, meaning the new enum case would fail to compile under Xcode 11. There is no compiler flag that I'm aware of to strip code based on the SDK or OS version being targeted.
At this time, we need to be able to build cleanly with Xcode 11 and 12. Ultimately, this is only temporary for some months until 12 becomes the default.
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.
Code changes look good.
There is an user comment about the #if swift(>=5.3)
that can be confusing, so maybe we can change it to #if compiler(>=5.3)
if it makes it less confusing, or add a comment before the line explaining it, and maybe a TODO to remove that code once we drop support of Xcode 11 next year.
@ikeith Thanks for the contribution to solve this problem of verisions, I have compiled my application in Capacitor again. |
This PR supersedes #3135 and #3149. It unifies the handling of permission enums and fixes the compiler errors under Xcode 12 with the iOS 14 SDK.