Relocate MainActor from class declaration

[kalebzen]

Hi there! We make use of Resolver for our dependency injection & service locator framework, and our SwiftMessages implementation involves registering our own instance of SwiftMessages as one of those dependencies.

When updating to the latest release (version 10.0.0), we are being required to annotate our dependency registration code with @MainActor due to the changes introduced in this commit. This ultimately requires us to make this change at the entry point of Resolver (which is executed at app launch), meaning we will end up registering all of our dependencies on the main thread even though that may not be necessary.

As mentioned on a separate issue in the Resolver repo - Even with MainActor annotations, there is no guarantee that the escaping closures will execute on the main thread, which could cause unexpected or inconsistent behavior. (Additional info on this subject if you're interested)

I totally understand the need for having SwiftMessages UI code get executed on the main thread, but I am less sure of the need to require it at the class-level as opposed to limiting it to the functions & properties that truly need to be on the main thread.

---

With that background out of the way - this PR is for relocating the MainActor annotation from the class level down into each function and property that the compiler requires it on. My implementation is admittedly naive, as I simply removed it from the class declaration and played whack-a-mole with the compiler until it was satisfied, so please let me know if I am making any invalid assumptions or if this may end up regressing the threading behavior that you were trying to fix to begin with.

[wtmoose]

Bro. Why aren't y'all using Factory yet

[wtmoose]

@kalebzen instead, would you be willing to wrap your instance in this?

class MainActorBox<T> {
    init(instance: @escaping @autoclosure @MainActor () -> T) {
        self.factory = instance
    }

    @MainActor
    var instance: T {
        let instance = _instance ?? factory()
        _instance = instance
        return instance
    }

    private let factory: @MainActor () -> T
    @MainActor private var _instance: T?
}

Usage:

@MainActor
struct Foo {}

let foo = MainActorBox { @MainActor in
    Foo()
}

Task { @MainActor in
    foo.instance
}

Introducing @MainActor helps avoid a common SwiftMessages error of calling the API from a background queue after an API call. I really don't like getting granular with the application of @MainActor. It just feels gross to me.

[wtmoose]

@kalebzen I noticed the striking header in doc center seems broken lately. Ya'll should check out our new library – I think it could fix that and eliminate a lot of code: https://github.com/SwiftKickMobile/SwiftUIMaterialTabs

[kalebzen]

Bro. Why aren't y'all using Factory yet

Hah I ask myself the same thing every month or so. Coincidentally we're actually looking into it (again) at the moment, hopefully 2.X will work for us. We initially began migrating to 1.X and found it didn't support our codebase very well so we punted on it.

instead, would you be willing to wrap your instance in this?

I appreciate the suggestion, I'll give it a shot!

I noticed the striking header in doc center seems broken lately. Ya'll should check out our new library – I think it could fix that and eliminate a lot of code: https://github.com/SwiftKickMobile/SwiftUIMaterialTabs

I'm interested to hear more - I'll reach out to you

---

As demonstrated in the last part of your example:

Task { @MainActor in
    foo.instance
}

To reference & return the underlying instance to Resolver, we will still have to swap into a MainActor context.

Unless we have our Resolver.registerAllServices() entry point annotated with @MainActor (thus forcing all registrations onto the main thread), at some point we will still have to include a Task { @MainActor in } in order to return an instance of MainActorBox/SwiftMessages to Resolver. Because our primary entry view/viewmodel injects SwiftMessages, wrapping its registration in a Task (MainActor or otherwise) consistently crashes at runtime due to the registration code not being invoked in time.

I agree that having to place MainActor this granularly isn't ideal, but I don't know that requiring all consumers to swap threads for initialization (which as far as I can tell doesn't invoke any UI code) is ideal either.

If your preference is to keep the MainActor the way it is to help ensure SwiftMessages is only ever being interfaced via the main thread, then we'll likely remain on 9.X until we can identify a path forward (perhaps this won't be a concern after swapping to Factory) 🙂

[wtmoose]

@kalebzen The box type I gave you would be returned by resolver without any need for main actor context. You would only need to be in a main context when accessing the instance property. You'd be doing that in a view, presumably, so I would think that solves your problem.

I wrote the sample code in a playground, which doesn't run in main actor context. So it demonstrates that:

1. You can instantiate the box type without being in a main context
2. Once in a main context, you can access the instance

Does that make sense or am I mis-understanding the issue?

[wtmoose]

Sample project:

MainActorBox.zip

I removed the @autoclosure from MainActorBox because the compiler was yelling at me and I didn't want to spend time figuring it out.

[wtmoose]

@kalebzen curious what your take is on the adapter type. Internal feedback I got was not positive, so I'm considering going with this PR. However, I like the adapter approach because it would work for any @MainActor type in general.

[kalebzen]

Does that make sense or am I mis-understanding the issue?

It does make sense - I think I was the one who misunderstood your original suggestion

@kalebzen curious what your take is on the adapter type. Internal feedback I got was not positive, so I'm considering going with this PR. However, I like the adapter approach because it would work for any @MainActor type in general.

I gave it a shot, it does seem to work. I agree it could be used in other spots, but until now we haven't had a use case for something like this.

I can't help but feel conflicted because it seems like placing MainActor on the class-level ends up shifting the onus of writing a small layer of abstraction like this onto many/all consumers of the library instead of SwiftMessages enforcing MainActor usage on the API's that truly need to be on the main thread. Unless I'm missing the benefits of requiring the class initializer to take place on the main thread, I think the library should ideally aim to impose as few restrictions as possible that are not absolutely necessary.

[wtmoose]

@kalebzen I'm planning to make this change. I've been swamped, so not sure exactly when. I'd stay on v9 for now unless you need something specific from v10.

[wtmoose]

@kalebzen sorry for being slow to release this. It occurred to me that, instead of your changes, we should be able to just make SwiftMessages.init() nonisolated. I've made that change on master. Could you let me know if this works for you?