square / Cleanse

Lightweight Swift Dependency Injection Framework
Other
1.78k stars 90 forks source link

How to solve dependency cycle / property injection doesn't seem to work. #98

Closed TomVanDerSpek closed 4 years ago

TomVanDerSpek commented 5 years ago

I'm having issues with dependency cycle. My Coordinator needs a LoginViewController and my LoginViewController needs a Coordinator to function. But I haven't found a way to get this to work with constructor injection. I also tried propertyInject, but the propertyInjection method doesn't get called. Below you'll see an example of my implementation.

protocol HomeRoute {
   func routeHome()
}

protocol RegisterRoute {
   func routeRegister()
}

class Coordinator: HomeRoute, RegisterRoute {

   private let loginViewControllerProvider: Provider<LoginViewController>

   init(loginViewControllerProvider: Provider<LoginViewController>) {
      self. loginViewControllerProvider = loginViewControllerProvider
   }

   func routeHome() {
      // show Home
   }

   func routeRegister() {
      // show Register
   }
}
class LoginViewController {
   typealias Coordinator = HomeRoute & RegisterRoute

   private var coordinator: Coordinator

   init(coordinator: Coordinator) {
       self.coordinator = coordinator
   }

   // Tryout with propertyInjection (is not being called)
   func injectProperties(_ coordinator: Coordinator) {
       self.coordinator = coordinator
   }
}
extension Modules {
    struct App: Cleanse.Module {
        static func configure(binder: AppBinder) {
            // Bind Coordinator (BTW is there a way to bind multiple variations of protocols to the same Coordinator? e.g. my RegisterCoordinator might only need a Coordinator confirming to HomeRoute protocol but should get the same Coordinator as the Login)
            binder.bind(LoginViewController.Coordinator.self).sharedInScope().to(factory: ApplicationCoordinator.init)

            binder.bind().to(factory: LoginViewController.init)

            // When running the app, this doesn't seem to do anything.
            binder.bindPropertyInjectionOf(LoginViewController.self).to(injector: LoginViewController.injectProperties) 

        }  
    } 
}
sebastianv1 commented 5 years ago

@TomVanDerSpek Circular dependencies should generally be broken up, but Cleanse does provide the WeakProvider in order to allow this (similar to Dagger's Lazy). So in your example, it looks like the LoginViewController should have a weak reference back up to the parent coordinator, so it should inject a WeakProvider<Coordinator> instance. And then calling .get() will provide you the actual instance of Coordinator.

import Cleanse

class LoginViewController {
   typealias Coordinator = HomeRoute & RegisterRoute

   private var coordinator: WeakProvider<Coordinator>

   init(coordinator: WeakProvider<Coordinator>) {
       self.coordinator = coordinator
   }

Regarding your issue with property injection, it doesn't look like you are calling injectProperties(into:) anywhere, which will populate your dependencies.

TomVanDerSpek commented 5 years ago

@sebastianv1 thank you for you answer, I would really like to solve the dependency cycle, but I don't think this is possible. The Coordinator is responsible for pushing the viewController and the viewController needs to notify the Coordinator that it should handle the navigation. So they need to know about each other. If you have any idea how to fix this I'd really like to know :)

Regarding the WeakProvider, unfortunately it is not possible to do this with protocol like you did in your example WeakProvider<Coordinator> Error: 'WeakProvider' requires that 'HotelRoute' be a class type Even though the protocol is of type AnyObject (protocol HotelRoute: AnyObject { ... }) When I change this to the Coordinator class instead of the typealias of protocols, and I call .get() on it, it returns nil

If I understand correctly, binder.bindPropertyInjectionOf(LoginViewController.self).to(injector: LoginViewController.injectProperties) is not enough for PI, but I don't know where I should/can callinjectProperties(into:).

sebastianv1 commented 5 years ago

Seems to be a limitation of the swift compiler to not see that the class constraint even if both protocols inherit from AnyObject. Either way, I've opened #99 which loosens the constraints on WeakProvider to accept non-class types (similar to how Provider works). How do you integrate Cleanse into your project? If it's CocoaPods I'll bump the podspec version and public a new version.

TomVanDerSpek commented 5 years ago

Yes Iā€™m using Cocoapods

sebastianv1 commented 5 years ago

Cool. Merged the fix so I'll bump the version and publish to trunk now.

TomVanDerSpek commented 5 years ago

Thanks for the quick fix! :)

Could you also help me out with the property injection? Or is it not possible to use property injection within a Module?

sebastianv1 commented 5 years ago

Sure thing! Essentially you need to grab the PropertyInjector instance you bound and call injectProperties(into:) on an instance of that type. (Please note this is more of a second class citizen and should only be used if you don't control construction of the object).

So in your example, since you're binding a property injector instance of LoginViewController, we would include that as a dependency in LoginViewController and then call it somewhere to populate our other dependencies. We could do it inside init, viewDidLoad, or at any point you desire to populate instances of those dependencies, but it should only be called once (I included it twice as example call sites)

class LoginViewController {
   typealias Coordinator = HomeRoute & RegisterRoute

   private var coordinator: Coordinator
   private let propertyInjector: PropertyInjector<LoginViewController>

   init(coordinator: Coordinator, propertyInjector: PropertyInjector<LoginViewController>) {
       self.coordinator = coordinator
       // We might be able to do it here if `self` works.
       propertyInjector.injectProperties(into: self) 
   }

   override func viewDidLoad() {
       super.viewDidLoad()
       // Or we could call it here.
       propertyInjector.injectProperties(into: self)
   }

   // Tryout with propertyInjection (is not being called)
   func injectProperties(_ coordinator: Coordinator) {
       self.coordinator = coordinator
   }
}

We've used property injection when dealing with storyboards (since UIKit handles actually calling init on those View controllers).

TomVanDerSpek commented 5 years ago

Ah that makes things clear, thanks!

TomVanDerSpek commented 5 years ago

@sebastianv1 ,

Unfortunately WeakProvider<LoginViewController.Coordinator> returns nil. Strangely enough when I specify a WeakProvider with a protocol it returns nil the first time I call .get() on it. When I specify a WeakProvder<Coordinator> where the Coordinator is the class with the protocol implementation, it doesn't return nil the first time. But it does return nil on every other object instantiated with WeakProvider<Coordinator> in its initialiser.

binder.bind().sharedInScope().to(factory: ApplicationCoordinator.init)

binder.bind().to(factory: LoginViewController.init)
binder.bind(LoginViewController.Interactor.self).to(factory: LoginInteractor.init)
binder.bind(LoginViewController.Coordinator.self).sharedInScope().to(factory: ApplicationCoordinator.init)

binder.bind().to(factory: LaunchViewController.init)
binder.bind(LaunchViewController.Interactor.self).to(factory: LaunchInteractor.init)
binder.bind(LaunchViewController.Coordinator.self).sharedInScope().to(factory: ApplicationCoordinator.init)
class LoginViewController {
    typealias Coordinator = HotelRoute

    init(interactor: Interactor, coordinator: WeakProvider<Coordinator>) {
        self.interactor = interactor
        self.coordinator = coordinator.get()
        super.init(nibName: nil, bundle: nil)
        interactor.presenter.delegate = self
    }
}
final class LaunchViewController: VIPViewController {
    typealias Coordinator = LaunchRoute

    init(interactor: Interactor, coordinator: WeakProvider< Coordinator >) {
        self.interactor = interactor
        self.coordinator = coordinator.get()
        super.init(nibName: nil, bundle: nil)
        interactor.presenter.delegate = self
    }
}
sebastianv1 commented 5 years ago

Hmm, I'm having a hard time reproducing this behavior on my end. Do you have a sample project that I can poke around?

TomVanDerSpek commented 5 years ago

Here is an example of the issue: https://github.com/TomVanDerSpek/cleanse-tryout-ios

sebastianv1 commented 4 years ago

This might be resolved by #122 . I'll clone down your sample project again and test it out, but PTAL!

sebastianv1 commented 4 years ago

Spoke too soon. Appears it does not. Will look into this further.

nugmanoff commented 4 years ago

@sebastianv1 Hey! Are there any updates on this one? I am having the similar case

nugmanoff commented 4 years ago

@TomVanDerSpek did you managed to solve your problem?

TomVanDerSpek commented 4 years ago

@nugmanoff no, unfortunately not.

nugmanoff commented 4 years ago

@TomVanDerSpek how did you move on with the issue then?

TomVanDerSpek commented 4 years ago

@nugmanoff I switch over to Swinject

nugmanoff commented 4 years ago

@TomVanDerSpek ok, thanks!

sebastianv1 commented 4 years ago

Hi all, I just pushed #131 which addresses this issue specifically. Very old issue, and validated using the sample project @TomVanDerSpek provided (had to also change retaining the ComponentFactory instance in the AppDelegate). Please take a look! :)

nugmanoff commented 4 years ago

@sebastianv1 that's great to hear! I was just considering which DI framework to use, and circular dependency (specifically the case that @TomVanDerSpek stumbled upon) - was one of the key factors :)

sebastianv1 commented 4 years ago

Closing as PR was merged into master. Please let me know if you all run into any further issues.

geraldeersteling commented 3 years ago

Just starting out with Cleanse.

Is this really fixed? I still get nil after trying out WeakProvider with a protocol... I have a circular VIP architecture in the form of:

VC -> Interactor Interactor -> Presenter Presenter -> VC (Weak) Router -> VC (Weak)

I use the WeakProvider in the initialisers of the Presenter and Router, but both of them give back a nil when trying to call get(). I.e. in the router:

weak var viewController: SearchGameViewController?
    var dataStore: SearchGameDataStore

    init(viewController: WeakProvider<SearchGameViewController>, dataStore: SearchGameDataStore) {
        self.viewController = viewController.get()
        self.dataStore = dataStore
        super.init()
    }

Am I doing something wrong or is this supposed to return nil? šŸ¤”

sebastianv1 commented 3 years ago

@geraldeersteling The bug should be fixed on master. Are you based off master or 4.2.5?

I had plans to release 4.2.6 soon with our new compile-time tool cleansec, but I'd be happy to just mark the tool as alpha and release 4.2.6 for you.

geraldeersteling commented 3 years ago

Used Cocoapods to install, iirc it's on 4.2.5.

But don't hurry yourself on my account šŸ˜„ I'm going to switch design-patterns to MVVM as we speak, so this cyclic dependency is not going to reappear there.