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.

Sign up for GitHub

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

Jump to bottom

Enable UIViewControllers in circular dependencies #47

Open
jakubvano opened this issue Apr 18, 2017 · 1 comment
Open

Enable UIViewControllers in circular dependencies #47

jakubvano opened this issue Apr 18, 2017 · 1 comment
Labels
enhancement

Comments

@jakubvano
Copy link
Member

jakubvano commented Apr 18, 2017 •
edited
Loading

Currently it is not possible to resolve in a standard way circular dependencies containing UIViewControllers from storyboard. A hack described in Swinject #245 gave me an idea: What if we "temporarily" registered an UIViewController instance during instantiation from storyboard?

Something like:

public func storyboardInitCompleted<C: Controller>(_ controllerType: C.Type, name: String? = nil, initCompleted: @escaping (Resolver, C) -> ()) {
    let factory = { (_: Resolver, controller: Controller) in controller }
    let wrappingClosure: (Resolver, Controller) -> () = { r, c in 
        (r as! Container).register(Controller.self) { _ in c } // Register an instantiated view controller to the container
        initCompleted(r, c as! C) 
    }
    let option = SwinjectStoryboardOption(controllerType: controllerType)
    _register(Controller.self, factory: factory, name: name, option: option)
        .initCompleted(wrappingClosure)
}

This might enable a standard way of resolving circular dependencies:

container.storyboardInitCompleted(ViewController.self) { r, vc in
    vc.presenter = r.resolve(Presenter.self)   
}

container.register(Presenter.self) { _ in Presenter() }
    .initCompleted { r, p in p.view = r.resolve(ViewController.self) }

Obviously, this would only work if ViewController is the entry point to object graph, and might have other restrictions. My fear is that providing a feature which resembles "normal" Swinject usage but has some restrictions might cause confusion for the users (but then again, there already are such restrictions in SwinjectStoryboard usage).
@yoichitgy What do you think? Is it worth pursuing something like this?
@yoichitgy
Copy link
Member

@jakubvano, it's a great idea👍 I think it's good to start implementing the feature. (Our principle is, as you know, we should keep Swinject main repo simple and reliable, but we can add more features to extension repos like SwinjectStoryboard.)

A couple of things I cared about:

  1. We expect a Resolver is a Container as (r as! Container). This is ok because in storyboardInitCompleted the resolver is always a Container.

  2. We might need to remove the temporary registration after calling initCompleted.

    let wrappingClosure: (Resolver, Controller) -> () = { r, c in 
        (r as! Container).register(Controller.self) { _ in c } // Register an instantiated view controller to the container
        initCompleted(r, c as! C) 
        // Should we remove the temporary registration here?
    }
  1. We should clarify the restrictions of SwinjectStoryboard usage in README later. In the example, Presenter should not be resolved manually, but only in storyboardInitCompleted closure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@yoichitgy @jakubvano