search

braze-inc / braze-swift-sdk

Braze SDK for the Apple ecosystem, including: iOS, macOS, iPadOS, visionOS, tvOS
https://www.braze.com
Other
52 stars 19 forks source link

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

Closed shagedorn closed 5 months ago

shagedorn commented 5 months ago

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 https://github.com/braze-inc/braze-swift-sdk/issues/85.

hokstuff commented 5 months ago

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!