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

Document how to adopt SafeDI in UIKit / AppDelegate apps #146

Closed
pouyayarandi opened this issue Jan 14, 2025 · 7 comments · Fixed by #148
Closed

Document how to adopt SafeDI in UIKit / AppDelegate apps #146

pouyayarandi opened this issue Jan 14, 2025 · 7 comments · Fixed by #148
Assignees
Labels
enhancement New feature or request

Comments

@pouyayarandi
Copy link

pouyayarandi commented Jan 14, 2025

Steps to reproduce

  1. Make AppDelegate as your root instantiable
  2. Generate SafeDI.swift

Sample code or sample project upload
Sample AppDelegate

import UIKit
import SafeDI
import UserRepo
import Networking

@Instantiable(isRoot: true)
@main
public class AppDelegate: UIResponder, UIApplicationDelegate, Instantiable {

    @Instantiated let viewControllerBuilder: Instantiator<ViewController>
    @Instantiated private let userRepository: UserRepository
    @Instantiated private let network: NetworkService

    public init(
        viewControllerBuilder: Instantiator<ViewController>,
        userRepository: UserRepository,
        network: NetworkService
    ) {
        self.viewControllerBuilder = viewControllerBuilder
        self.userRepository = userRepository
        self.network = network
    }

    public var window: UIWindow?

    public func application(
        _ application: UIApplication,
        didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?
    ) -> Bool {
        window = .init(frame: UIScreen.main.bounds)
        window?.rootViewController = viewControllerBuilder.instantiate()
        window?.makeKeyAndVisible()
        return true
    }
}

Expected behavior
It compiles successfully.

Actual behavior
It has two issues:

  1. In generated code override is missing.
extension AppDelegate {
    public convenience init() { // compile error: Overriding declaration requires an 'override' keyword
        func __safeDI_network() -> NetworkServiceImpl {
            let session: URLSession = URLSession.instantiate()
            let decoder: JSONDecoder = JSONDecoder.instantiate()
            return NetworkServiceImpl(session: session, decoder: decoder)
        }
        let network: NetworkService = __safeDI_network()
        func __safeDI_viewControllerBuilder() -> ViewController {
            let userRepository: UserRepository = UserRepositoryImpl(network: network)
            return ViewController(userRepository: userRepository)
        }
        let viewControllerBuilder = Instantiator<ViewController>(__safeDI_viewControllerBuilder)
        let userRepository: UserRepository = UserRepositoryImpl(network: network)
        self.init(viewControllerBuilder: viewControllerBuilder, userRepository: userRepository, network: network)
    }
}
  1. Xcode complains as init() is already overriden.
Screenshot 2025-01-14 at 11 48 56 PM
@pouyayarandi pouyayarandi added the bug Something isn't working label Jan 14, 2025
@dfed
Copy link
Owner

dfed commented Jan 14, 2025

Interesting. I haven't made an AppDelegate the root before. Last time I converted a UIKit-root app I made the root an object that the AppDelegate created and retained, which is an easy workaround.

This is a bit tricky because the generated initializer in this case would need to be both an override and a convenience initializer, and we don't necessarily have enough information at the @Instantiable-decorated implementation to know whether the superclass defines an init() (and whether it is decorated with required).

That said, I'm going to leave this open as a feature request. I can imagine adding a rootInitModifiers parameter to the @Instantiable macro where you could define what modifiers you need a generated init to have.

@dfed dfed added enhancement New feature or request and removed bug Something isn't working labels Jan 14, 2025
@dfed dfed self-assigned this Jan 14, 2025
@dfed dfed changed the title Generated code fails when root Instantiable is AppDelegate. Enable adding declaration modifiers to the generated init() on a root type Jan 14, 2025
@pouyayarandi
Copy link
Author

pouyayarandi commented Jan 14, 2025

Another workaround I used:
I changed generated init() to static func instantiate() -> AppDelegate

Then, I added another init in my AppDelegate.swift file which initiate it from another AppDelegate

extension AppDelegate {
    convenience init(_ a: AppDelegate) {
        self.init(
            viewControllerBuilder: a.viewControllerBuilder,
            userRepository: a. userRepository,
            network: a.network
        )
    }
}

Finally, I used it inside AppDelegate to implement init()

public convenience override init() {
    self.init(.instantiate())
}

So, if I had an option in safe-di-tool to generate static func instantiate instead of init() I could handle it.

safe-di-tool --generate-instantiate-function

@dfed
Copy link
Owner

dfed commented Jan 14, 2025

I think I prefer adding modifiers to the exiting init since that's not a breaking change, but the approach of creating an instantiate() method is certainly clever 🙂

@dfed
Copy link
Owner

dfed commented Jan 15, 2025

Valid combinations of decorators we need to care about are:

required convenience init() // when the superclass's `init()` is `required`
override convenience init() // when the superclass defines an `init()` without the `required` decorator

I think that's it. You can imagine updating our @Instantiable macro to read:

public macro Instantiable(
    isRoot: Bool = false,
    superclassHasNoParameterInit: Bool = false,
    superclassHasRequiredNoParameterInit: Bool = false,
    fulfillingAdditionalTypes additionalTypes: [Any.Type] = [],
    conformsElsewhere: Bool = false
)

And then we can make the macro enforce that you don't set both superclassHasNoParameterInit and superclassHasRequiredNoParameterInit to true since that won't compile. Add some good parameter documentation, and we might be golden.

I don't love this API, but I'm struggling to come up with an alternative that is:

  • Easy to parse
  • Supports the different configurations we need
  • Is not a breaking change

I am open to suggestions!

@pouyayarandi
Copy link
Author

It fixes the first problem, but I have no idea about the second one. Does it fix the "already overriden" problem in AppDelegate?

@dfed
Copy link
Owner

dfed commented Jan 15, 2025

Great point. I just played around a little and I think you're right that this still won't work for working with Objective-C objects.

Here's the error I get when utilizing an AppDelegate (which inherits its init() from UIResponder):

@main
public class AppDelegate: UIResponder, UIApplicationDelegate {
    public init(test: String) {}
}

extension AppDelegate {
    public override convenience init() { // Error: Multiple definitions of symbol '$s14SampleProject11AppDelegateCACycfc'
        self.init(test: "hi")
    }
}

And then here's an error I got playing with another Objective-C inherited type:

public class SuperTest: NSObjectProtocol {
    public init() {}

    // stub NSObjectProtocol implementation here…
}

public final class Test: SuperTest {
    public init(test: String) {}
}

extension Test {
    public override convenience init() { // Error: Non-@objc initializer 'init()' declared in 'SuperTest' cannot be overridden from extension
        self.init(test: "test")
    }
}

So, maybe this feature isn't worth adding? The additional complexity in @Instantiated we'd be taking on would only help when extending subclasses of Swift objects, not Objective-C objects, and it seems like a rare case that a pure Swift object that's a subclass would be made the root DI object. Meanwhile, the workaround for "an Objective-C object is the root of my binary but not the root of my dependency tree" is simply "the Objective-C object creates the root of the dependency tree".

I think this workaround merits documentation. I'm going to update the title of this issue to track that proposed resolution.

@dfed dfed changed the title Enable adding declaration modifiers to the generated init() on a root type Document how to adopt SafeDI in UIKit apps Jan 15, 2025
@dfed
Copy link
Owner

dfed commented Jan 15, 2025

I'll add a subheader under the Migrating to SafeDI section of our documentation. This section may also mention a suggested root for SwiftUI apps.

@dfed dfed changed the title Document how to adopt SafeDI in UIKit apps Document how to adopt SafeDI in UIKit / AppDelegate apps Jan 16, 2025
@dfed dfed linked a pull request Jan 16, 2025 that will close this issue
@dfed dfed closed this as completed Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants