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

[Firestore] Label '@MainActor' isolated APIs of 'FirestoreQuery' #14163

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented Nov 22, 2024

This PR addresses the following errors in Swift 6 mode:

// FirestoreQuery.swift
@available(iOS 14.0, macOS 11.0, macCatalyst 14.0, tvOS 14.0, watchOS 7.0, *)
@propertyWrapper
public struct FirestoreQuery<T>: DynamicProperty {
  @StateObject private var firestoreQueryObservable: FirestoreQueryObservable<T>

// <--snip-->

  public var wrappedValue: T {
    firestoreQueryObservable.items // 💥 Main actor-isolated property 'firestoreQueryObservable' can not be referenced from a nonisolated context
  }

  /// A binding to the request's mutable configuration properties
  @MainActor @preconcurrency public var projectedValue: Configuration {
    get {
      firestoreQueryObservable.configuration // 💥 Main actor-isolated property 'firestoreQueryObservable' can not be referenced from a nonisolated context
    }
    nonmutating set {
      firestoreQueryObservable.objectWillChange.send() // 💥 Main actor-isolated property 'firestoreQueryObservable' can not be referenced from a nonisolated context
      firestoreQueryObservable.configuration = newValue // 💥 Main actor-isolated property 'firestoreQueryObservable' can not be referenced from a nonisolated context
    }
  }

// <--snip-->

}

The issue is that firestoreQueryObservable is decorated with @StateObject which is labeled @MainActor in Xcode 16.1:

@MainActor @frozen @propertyWrapper @preconcurrency public struct StateObject<ObjectType> : DynamicProperty where ObjectType : ObservableObject { ... }

So the property is isolated to the main actor and cannot be referenced from a non-main-actor-isolated context.

Backwards compatibility

I found that adding @MainActor causes a warning in Xcode 16+ for cases where @FirestoreQuery is used inside a non-isolated class. The warning is not ideal (and a little unexpected, see https://www.jessesquires.com/blog/2024/06/05/swift-concurrency-non-sendable-closures/), but I think it's unlikely developers will face them as the @FirestoreQuery is likely going to be used in a SwiftUI view which has the correct @MainActor isolation by default.

For Xcode 15, this PR doesn't seem to make a difference in behavior.

class MyNonIsolatedClass {
  @FirestoreQuery(
    collectionPath: "fruits",
    predicates: [
      .where("isFavourite", isEqualTo: true),
    ]
  ) fileprivate var fruitResults: [Fruit]
  
  nonisolated func nonisolatedFunc() {
    // Xcode 15.2
    // Doesn't compile currently with or without `@MainActor @preconcurrency`
    // 💥 Main actor-isolated property 'fruitResults' can not be referenced from a nonisolated context
    
    // Xcode 16.1
    // Currently, without this PR's `@MainActor @preconcurrency`, this PR causes a warning:
    // ⚠️ Main actor-isolated property 'fruitResults' can not be referenced from a nonisolated context; this is an error in the Swift 6 language mode
    // Without @preconcurrency, this PR causes an error:
    // 💥 Main actor-isolated property 'fruitResults' can not be referenced from a nonisolated context
    // With @preconcurrency, this PR causes a warning:
    // ⚠️ Main actor-isolated property 'fruitResults' can not be referenced from a nonisolated context; this is an error in the Swift 6 language mode
    
    // Xcode 16.2 b2
    // Same behavior as 16.1

    print(fruitResults)
  }
  
  func defaultIsolationFunc() {
    // Xcode 15.2
    // No error/warn, with @preconcurrency or not
    
    // Xcode 16.1
    // Same as Xcode 15.2

    // Xcode 16.2 b2
    // Same as Xcode 16.1

    print(fruitResults)
  }
}

#no-changelog

Copy link
Contributor

Apple API Diff Report

Commit: ff8b117
Last updated: Fri Nov 22 08:34 PST 2024
View workflow logs & download artifacts


[BUILD ERROR] FirebaseFirestore


@ncooke3 ncooke3 merged commit 01dc420 into main Dec 11, 2024
35 checks passed
@ncooke3 ncooke3 deleted the nc/fst-mainactor branch December 11, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants