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

[Bug]: Adopt Swift Concurrency annotations in public APIs #132

Closed
shagedorn opened this issue Jun 21, 2024 · 1 comment
Closed

[Bug]: Adopt Swift Concurrency annotations in public APIs #132

shagedorn opened this issue Jun 21, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@shagedorn
Copy link

Platform

iOS

Platform Version

Braze SDK Version

9.3.0

Xcode Version

Xcode 16 beta (or earlier with Swift feature flags)

Computer Processor

Apple (M1)

Repro Rate

100% (compile-time)

Steps To Reproduce

The Braze SDK is lacking Swift Concurrency annotations, especially in its delegates.

We e.g. assume that func braze(:shouldOpenURL:) -> Bool runs on main, given its apparent relation to the respective UIApplication and UISceneDelegate functions, which are all annotated as @MainActor. Similarly, func braze(:willPresentModalWithContext:) and many more are obviously UI-related and therefore main-bound in all likelihood, but especially with Xcode 16 and/or Concurrency-related Swift feature flags enabled, the lack of annotations is starting to become a problem.

Implementing e.g. BrazeDelegate in our app within an instance that itself is annotated as @MainActor raises the following warning in Xcode 16:

Main actor-isolated instance method 'braze(_:shouldOpenURL:)' cannot be used to satisfy nonisolated protocol requirement; this is an error in the Swift 6 language mode

FixIt suggests 2 options:

image

Option 1 isn't feasible because we have code paths in there that do expect to run on main, and to satisfy the compiler, we'd have to introduce asynchronous calls, which doesn't really work given the delegate method expects a synchronous return value.

Option 2 is an ok intermediate solution and we'll do that for now, but Swift now has (and has had for a while) tools to turn runtime assumptions into compile-time-checked guarantees, and I believe these should be used – especially since callsites now have to take action (adding @preconcurrency) to avoid warnings.

Expected Behavior

We believe that the mentioned APIs (and probably many more) should be annotated as @MainActor, and generally, the entire public API should be reviewed for Swift Concurrency support.

Actual Incorrect Behavior

No annotations are present, leading to compiler warnings when callsites have adopted Swift Concurrency.

Verbose Logs

No response

Additional Information

This is loosely related to #85.

@shagedorn shagedorn added the bug Something isn't working label Jun 21, 2024
@hokstuff
Copy link
Collaborator

Hi @shagedorn,

Thank you for raising this with detailed descriptions of what you're seeing. Since this issue also has the same root request as the related issue #85, I will mark it as a duplicate and link it to that issue.

We currently have allocated work to make the entire SDK to support Swift Concurrency, and there are many areas throughout our SDK(s) that require refactoring to support it. We have noted your request and will prioritize accordingly.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants