Swinject / SwinjectAutoregistration

Swinject extension to automatically register your services
MIT License
251 stars 45 forks source link

Autoregistration for property injection #57

Closed apstygo closed 4 years ago

apstygo commented 4 years ago

SwinjectAutoregistration is awesome and is a perfect showcase of Swift's generics capabilities. But sadly it omits property injection 😕.

I've got a couple of use cases for such a feature in my codebase, so I decided to try and extend Container myself. It turned out quite straightforward actually.

@discardableResult
    func autoregister<Service, T>(initializer: @escaping () -> Service,
                                  property: WritableKeyPath<Service, T>) -> ServiceEntry<Service> {
        return register(Service.self, factory: { res in
            let arg = res.resolve(T.self)!
            var service = initializer()
            service[keyPath: property] = arg
            return service
        })
    }

What do you think? Should I turn this into a full-fledged PR with methods for more properties?

[Update] Here's the use case:

/// Registration
container.autoregister(initializer: PinViewController.init, property: \.viewModel)

/// Resolving
@objc private func handleLogoTap() {
    let vc = DI.shared.container.resolve(PinViewController.self)!
    present(UINavigationController(rootViewController: vc), animated: true)
}
timqzm commented 4 years ago

Probably you can find my autoInitCompleted func implementation also suitable for your case 😉

timqzm commented 4 years ago

@apstygo, see:

container.autoregister(initializer: PinViewController.init)
    .autoInitCompleted(PinViewController.setup)

class PinViewController: UIViewController {
    private var viewModel: PinViewModel?

    func setup(with viewModel: PinViewModel) {
        self.viewModel = viewModel
    }
}
apstygo commented 4 years ago

@timqzm, this seems like a rather indirect way of going about property injection. It requires editing the service itself for the purposes of DI, which I personally don't like. Moreover, the methods I propose are (I hope) inline with how the rest of SwinjectAutoregistration works.

timqzm commented 4 years ago

That's true. Does your example work if viewModel is private?

tkohout commented 4 years ago

@apstygo Thanks for the contribution!

I like the idea. I imagine something like following function would be generated for more than one injected property:

func autoregister<Service, A, B, C, D>(_ service: Service.Type, initializer: @escaping () -> Service,
                                           properties a: WritableKeyPath<Service, A>, _ b: WritableKeyPath<Service, B>, _ c: WritableKeyPath<Service, C>, _ d: WritableKeyPath<Service, D>) -> ServiceEntry<Service> {
//....
}

and then calling like this would be pretty cool.

container.autoregister(ServiceX.self, initializer: ServiceX.init, properties: \.dependencyA, \.dependencyB, \.dependencyC, \.dependencyD)

Not using property injection myself - but assuming - you would always have an initializer with zero arguments.

apstygo commented 4 years ago

@tkohout The generation part is exactly what I was thinking about! And as for the method signature, I kinda wonder if we need the service parameter at all. If we are gonna go down this route, then we'll have to generate methods for all possible combinations of arguments and properties, which is a lot of methods. Maybe stick to just the property injection? For combinations of initializer and property injection one could always fallback to the old way of calling register.

tkohout commented 4 years ago

Maybe stick to just the property injection - yes, that was my point The reason you have to specify the Service with a type is that more often than not you want to register the concrete service under a protocol and not its concrete type.

All autoregistration code is generated in bin/generate script. Would be great to have the code to property registration also been generated this way.

I am looking forward to your pull request 👍

apstygo commented 4 years ago

The reason you have to specify the Service with a type is that more often than not you want to register the concrete service under a protocol and not its concrete type.

It seems to me that we've run into a problem. To the method both base service and concrete service are going to have the same signature. This is not a problem for initializer injection, as we give it enough information by providing the relevant initializer. But with property injection all we're going to get is public API of the base service, not the concrete service, which may not always be enough to set the properties.

tkohout commented 4 years ago

You are absolutely right. It's getting late here but my quick thoughts to this:

While you could do something like this:

@discardableResult
    func autoregister<ConcreteService, Service, A>(_ type: Service.Type, initializer: @escaping () -> ConcreteService, properties a: WritableKeyPath<ConcreteService, A>) -> ServiceEntry<Service>  {

        return register(type.self, factory: { res in
            var service = initializer()
            service[keyPath: a] = res.resolve(A.self)!
            return service as! Service
        })
    }

To my best knowledge you can't do where ConcreteService: Service as you need to conform to a protocol or class, not another generic. Second best thing would be to do a check at registration time, but again, I don't think that's currently possible in swift. You can only check whether an instance conforms to a type at resolution time which would make this pretty much type unsafe.