Skip to content

Swift 6: complete concurrency checking #825

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

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
ddf78cc
Use SWIFT_STRICT_CONCURRENCY complete with Swift 5
laevandus Mar 28, 2025
b73c9c5
Merge branch 'develop' into swift-6
laevandus May 2, 2025
2f64d75
Make view models MainActor and fix isolation for controller delegates…
laevandus May 2, 2025
87292cf
Set Swift version to 6
laevandus May 6, 2025
ab7356d
Constraint and UIDevice errors
laevandus May 6, 2025
391b156
Try MainActor Appearance and non-isolated injected values
laevandus May 6, 2025
26dd6aa
Upgrade Nuke to 12.8
laevandus May 6, 2025
ef3bbc0
Fixed all the errors
laevandus May 7, 2025
bb0ed90
Fix last warnings
laevandus May 7, 2025
87fcb5a
Fix Swift 6 warnings in tests
laevandus May 7, 2025
994196f
Disable building with Xcode 15
laevandus May 7, 2025
4c94da4
Fix tests
laevandus May 7, 2025
6b78288
Fix tests
laevandus May 8, 2025
8eae0dd
Merge branch 'develop' into swift-6
laevandus May 8, 2025
6e1e8c9
Fix build warnings in the demo app
laevandus May 8, 2025
f10725c
Fix avatar sizes with LazyImage
laevandus May 8, 2025
c0b9111
Fix UI test where LazyImage now is treated as images, not otherElements
laevandus May 9, 2025
79ef1c1
Tidy here and there
laevandus May 9, 2025
e28503d
Use Swift 6 in the demo app
laevandus May 9, 2025
f8d9504
Merge branch 'develop' into swift-6
laevandus May 9, 2025
fa2872a
Fix runtime asserts
laevandus May 9, 2025
4b305c5
Fix Gif support which LazyImage dropped
laevandus May 9, 2025
1ab09e2
Merge branch 'develop' into swift-6
laevandus May 22, 2025
1170a52
Do not change view factory method signatures (opt-out from checking)
laevandus May 22, 2025
7a0cfe3
Rename MainActor.ensureIsolated to StreamConcurrency.onMain
laevandus May 23, 2025
7d0237c
Fix giphy image lookup since it is image directly (changes in Nuke)
laevandus May 23, 2025
9769ff4
Remove assumeIsolated because Swift 6 compiler does not require it
laevandus May 23, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/smoke-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ jobs:
build-xcode15:
name: Build SDKs (Xcode 15)
runs-on: macos-15
if: ${{ github.event.inputs.record_snapshots != 'true' }}
#if: ${{ github.event.inputs.record_snapshots != 'true' }}
if: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Xcode 15 builds are going away, disabling it for now and a cleanup happens separately

env:
XCODE_VERSION: "15.4"
steps:
Expand Down
2 changes: 1 addition & 1 deletion .swiftformat
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Stream rules
--header "\nCopyright © {year} Stream.io Inc. All rights reserved.\n"
--swiftversion 5.9
--swiftversion 6.0

--ifdef no-indent
--disable redundantType
Expand Down
2 changes: 1 addition & 1 deletion DemoAppSwiftUI/AppConfiguration/AppConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import StreamChat
import StreamChatSwiftUI

final class AppConfiguration {
static let `default` = AppConfiguration()
@MainActor static let `default` = AppConfiguration()

/// The translation language to set on connect.
var translationLanguage: TranslationLanguage?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,17 @@ import SwiftUI
struct AppConfigurationTranslationView: View {
@Environment(\.dismiss) var dismiss

var selection: Binding<TranslationLanguage?> = Binding {
AppConfiguration.default.translationLanguage
} set: { newValue in
AppConfiguration.default.translationLanguage = newValue
}

var body: some View {
List {
ForEach(TranslationLanguage.all, id: \.languageCode) { language in
Button(action: {
selection.wrappedValue = language
AppConfiguration.default.translationLanguage = language
dismiss()
}) {
HStack {
Text(language.languageCode)
Spacer()
if selection.wrappedValue == language {
if AppConfiguration.default.translationLanguage == language {
Image(systemName: "checkmark")
}
}
Expand Down
4 changes: 2 additions & 2 deletions DemoAppSwiftUI/AppleMessageComposerView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ struct AppleMessageComposerView<Factory: ViewFactory>: View, KeyboardReadable {
}
)
.offset(y: -composerHeight)
.animation(nil) : nil,
.animation(.none, value: viewModel.showCommandsOverlay) : nil,
alignment: .bottom
)
.modifier(factory.makeComposerViewModifier())
Expand Down Expand Up @@ -214,7 +214,7 @@ struct BlurredBackground: View {
}

struct HeightPreferenceKey: PreferenceKey {
static var defaultValue: CGFloat? = nil
static let defaultValue: CGFloat? = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this causing issues somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PreferenceKey's protocol requires the default value to be get, therefore this is fine to be defined with let over mutable var (which causes concurrency errors).
Docs


static func reduce(value: inout CGFloat?, nextValue: () -> CGFloat?) {
value = value ?? nextValue()
Expand Down
14 changes: 9 additions & 5 deletions DemoAppSwiftUI/ChannelHeader/BlockedUsersViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import StreamChat
import StreamChatSwiftUI
import SwiftUI

class BlockedUsersViewModel: ObservableObject {
@MainActor class BlockedUsersViewModel: ObservableObject {

@Injected(\.chatClient) var chatClient

Expand All @@ -27,8 +27,10 @@ class BlockedUsersViewModel: ObservableObject {
} else {
let controller = chatClient.userController(userId: blockedUserId)
controller.synchronize { [weak self] _ in
if let user = controller.user {
self?.blockedUsers.append(user)
Task { @MainActor in
if let user = controller.user {
self?.blockedUsers.append(user)
}
}
}
}
Expand All @@ -39,8 +41,10 @@ class BlockedUsersViewModel: ObservableObject {
let unblockController = chatClient.userController(userId: user.id)
unblockController.unblock { [weak self] error in
if error == nil {
self?.blockedUsers.removeAll { blocked in
blocked.id == user.id
Task { @MainActor in
self?.blockedUsers.removeAll { blocked in
blocked.id == user.id
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion DemoAppSwiftUI/ChannelHeader/ChooseChannelQueryView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import StreamChatSwiftUI
import SwiftUI

struct ChooseChannelQueryView: View {
static let queryIdentifiers = ChannelListQueryIdentifier.allCases.sorted(using: KeyPathComparator(\.title))
static var queryIdentifiers: [ChannelListQueryIdentifier] {
ChannelListQueryIdentifier.allCases.sorted(by: { $0.title < $1.title })
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was giving Sendable related error which did not feel right. Just switched to another sorting method.


var body: some View {
ForEach(Self.queryIdentifiers) { queryIdentifier in
Expand Down
40 changes: 24 additions & 16 deletions DemoAppSwiftUI/ChannelHeader/NewChatViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import StreamChat
import StreamChatSwiftUI
import SwiftUI

class NewChatViewModel: ObservableObject, ChatUserSearchControllerDelegate {
@MainActor class NewChatViewModel: ObservableObject, ChatUserSearchControllerDelegate {

@Injected(\.chatClient) var chatClient

Expand Down Expand Up @@ -99,31 +99,37 @@ class NewChatViewModel: ObservableObject, ChatUserSearchControllerDelegate {
if !loadingNextUsers {
loadingNextUsers = true
searchController.loadNextUsers(limit: 50) { [weak self] _ in
guard let self = self else { return }
self.chatUsers = self.searchController.userArray
self.loadingNextUsers = false
Task { @MainActor in
guard let self = self else { return }
self.chatUsers = self.searchController.userArray
self.loadingNextUsers = false
}
}
}
}

// MARK: - ChatUserSearchControllerDelegate

func controller(
nonisolated func controller(
Copy link
Contributor

Choose a reason for hiding this comment

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

can we avoid this being nonisolated in the LLC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nonisolated is required because I made the view model @MainActor which I feel like makes sense to have. Controllers have a design where I can set any queue to be the one used for delegate callbacks. In 99% of cases it is the default main queue (highly unlikely that many set their own queues). But because we support any queue for delegate callbacks, we can't make the delegate protocol @MainActor. We can't use @preconcurrency for the delegate conformance because then we would get runtime crash if anyone change the the default queue in the controller to be a background queue.
Ideally we would use our async-await state layer instead of delegates in the future, but that is out of scope.

Summary is that nonisolated can be avoided if view models are not @MainActor (I currenty changed them to be). That said, in the UIKit implementation view controllers are implementing delegates, so nonisolated is required there because UIViewController is @MainActor (similar situation). Similar issue with completion handlers where the calling thread can be any thread.

_ controller: ChatUserSearchController,
didChangeUsers changes: [ListChange<ChatUser>]
) {
chatUsers = controller.userArray
Task { @MainActor in
chatUsers = controller.userArray
}
}

// MARK: - private

private func searchUsers(with term: String?) {
state = .loading
searchController.search(term: term) { [weak self] error in
if error != nil {
self?.state = .error
} else {
self?.state = .loaded
Task { @MainActor in
if error != nil {
self?.state = .error
} else {
self?.state = .loaded
}
}
}
}
Expand All @@ -137,13 +143,15 @@ class NewChatViewModel: ObservableObject, ChatUserSearchControllerDelegate {
extraData: [:]
)
channelController?.synchronize { [weak self] error in
if error != nil {
self?.state = .error
self?.updatingSelectedUsers = false
} else {
withAnimation {
self?.state = .channel
Task { @MainActor in
if error != nil {
self?.state = .error
self?.updatingSelectedUsers = false
} else {
withAnimation {
self?.state = .channel
self?.updatingSelectedUsers = false
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion DemoAppSwiftUI/CreateGroupView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ struct SearchBar: View {
}
.padding(.trailing, 10)
.transition(.move(edge: .trailing))
.animation(.default)
.animation(.default, value: isEditing)
}
}
}
Expand Down
28 changes: 17 additions & 11 deletions DemoAppSwiftUI/CreateGroupViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import StreamChat
import StreamChatSwiftUI
import SwiftUI

class CreateGroupViewModel: ObservableObject, ChatUserSearchControllerDelegate {
@MainActor class CreateGroupViewModel: ObservableObject, ChatUserSearchControllerDelegate {

@Injected(\.chatClient) var chatClient

Expand Down Expand Up @@ -75,10 +75,12 @@ class CreateGroupViewModel: ObservableObject, ChatUserSearchControllerDelegate {
members: Set(selectedUsers.map(\.id))
)
channelController.synchronize { [weak self] error in
if error != nil {
self?.errorShown = true
} else {
self?.showGroupConversation = true
Task { @MainActor in
if error != nil {
self?.errorShown = true
} else {
self?.showGroupConversation = true
}
}
}

Expand All @@ -89,22 +91,26 @@ class CreateGroupViewModel: ObservableObject, ChatUserSearchControllerDelegate {

// MARK: - ChatUserSearchControllerDelegate

func controller(
nonisolated func controller(
_ controller: ChatUserSearchController,
didChangeUsers changes: [ListChange<ChatUser>]
) {
chatUsers = controller.userArray
Task { @MainActor in
chatUsers = controller.userArray
}
}

// MARK: - private

private func searchUsers(with term: String?) {
state = .loading
searchController.search(term: term) { [weak self] error in
if error != nil {
self?.state = .error
} else {
self?.state = .loaded
Task { @MainActor in
if error != nil {
self?.state = .error
} else {
self?.state = .loaded
}
}
}
}
Expand Down
18 changes: 10 additions & 8 deletions DemoAppSwiftUI/DemoAppSwiftUIApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ struct DemoAppSwiftUIApp: App {
}
}

class AppState: ObservableObject, CurrentChatUserControllerDelegate {
@MainActor class AppState: ObservableObject, CurrentChatUserControllerDelegate {
@Injected(\.chatClient) var chatClient: ChatClient

// Recreate the content view when channel query changes.
Expand Down Expand Up @@ -123,13 +123,15 @@ class AppState: ObservableObject, CurrentChatUserControllerDelegate {
contentIdentifier = identifier.rawValue
}

func currentUserController(_ controller: CurrentChatUserController, didChangeCurrentUserUnreadCount: UnreadCount) {
unreadCount = didChangeCurrentUserUnreadCount
let totalUnreadBadge = unreadCount.channels + unreadCount.threads
if #available(iOS 16.0, *) {
UNUserNotificationCenter.current().setBadgeCount(totalUnreadBadge)
} else {
UIApplication.shared.applicationIconBadgeNumber = totalUnreadBadge
nonisolated func currentUserController(_ controller: CurrentChatUserController, didChangeCurrentUserUnreadCount: UnreadCount) {
Task { @MainActor in
unreadCount = didChangeCurrentUserUnreadCount
let totalUnreadBadge = unreadCount.channels + unreadCount.threads
if #available(iOS 16.0, *) {
UNUserNotificationCenter.current().setBadgeCount(totalUnreadBadge)
} else {
UIApplication.shared.applicationIconBadgeNumber = totalUnreadBadge
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion DemoAppSwiftUI/DemoUser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public let apiKeyString = "zcgvnykxsfm8"
public let applicationGroupIdentifier = "group.io.getstream.iOS.ChatDemoAppSwiftUI"
public let currentUserIdRegisteredForPush = "currentUserIdRegisteredForPush"

public struct UserCredentials: Codable {
public struct UserCredentials: Codable, Sendable {
public let id: String
public let name: String
public let avatarURL: URL?
Expand Down
2 changes: 1 addition & 1 deletion DemoAppSwiftUI/LaunchAnimationState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import SwiftUI

class LaunchAnimationState: ObservableObject {
@MainActor class LaunchAnimationState: ObservableObject {

@Published var showAnimation = true

Expand Down
1 change: 0 additions & 1 deletion DemoAppSwiftUI/LoginView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ struct LoginView: View {
DemoUserView(user: user)
}
.padding(.vertical, 4)
.animation(nil)
}
.listStyle(.plain)

Expand Down
6 changes: 3 additions & 3 deletions DemoAppSwiftUI/LoginViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import StreamChat
import StreamChatSwiftUI
import SwiftUI

class LoginViewModel: ObservableObject {
@MainActor class LoginViewModel: ObservableObject {

@Published var demoUsers = UserCredentials.builtInUsers
@Published var loading = false
Expand Down Expand Up @@ -42,7 +42,7 @@ class LoginViewModel: ObservableObject {
return
}

DispatchQueue.main.async { [weak self] in
Task { @MainActor [weak self] in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DispatchQueue gives Sendable error for self

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we should use Task going forward

withAnimation {
self?.loading = false
UnsecureRepository.shared.save(user: credentials)
Expand All @@ -64,7 +64,7 @@ class LoginViewModel: ObservableObject {
return
}

DispatchQueue.main.async { [weak self] in
Task { @MainActor [weak self] in
withAnimation {
self?.loading = false
AppState.shared.userState = .loggedIn
Expand Down
Loading
Loading