Dependency injection

[marciniwanicki]

Is your feature request related to a problem? Please describe.
Passing an object from the client code to the classes that are located lower in the call-chain requires quite a bit of daisy chaining. Due to that, some of the standard dependencies might not be propagated from the top to bottom, for instance Logging cannot be replaced globally. Having a DI / DI-like solution would ease the process of introducing new components i.e. Metrics.

Describe the solution you'd like
Our class structure is quite small, we could probably fit all the initialization of a module into a single Factory class that lazy-instantiates all the non-data objects within that module.
We would need to remove default init parameters to make sure all the dependencies are passed from the factory.
It can also make the testing a bit easier, we wouldn't need to create individual mock instances in each TestCase class.

Describe alternatives you've considered

• Swinject, it's a great lib, not sure if it's worth pulling an external dependency just yet

Desktop (please complete the following information):

• OS: macOS
• Version: master (7935c55)

[marciniwanicki]

I've drafted a small prototype for TuistGenerator.
https://github.com/bloomberg/tuist/tree/spike/MI/351-dependency-injection-via-plain-factory-method

Some positive points 🙂:

• Easier to shared common objects, like printer, fileHandler, should be also easier to inject new dependencies in the future or wrap existing by proxy (i.e. to log their performance).
• No need to have more convenient initializers, each class has just one internal initializer with no default parameters.
• Unit tests are now very explicit and clear that some of them test the subject and integration with some of its dependencies (when they're not mocked).

On the negative side 😐:

• Having all the creation code in one place is not easy to read
• Set up code in the unit tests is a bit longer
• I don't think we need it now, but if so, this solution won't resolve graph-wiring when we need to share an instance between a given set of objects, and create a new one for others.

[pepicrft]

I agree with you that passing all those dependencies down to every class is a bit nasty. I'm not fully convinced by the idea of having a factory though. I think it adds more overhead than the value that it brings. I'd understand having a factory if we are generating objects with a similar interface but with different implementations (if I'm not wrong I think that's where the concept of the factory comes from).

Here's another approach:

protocol FileHandling {}
protocol Logging {}

class Context {}
extension Context: FileHandling {}
extension Context: Logging{}

class Command {
  let context: FileHandling && Logging

  init(context: FileHandling && Logging) {
    self.context = context
  }
}

Maybe context is not the right name. The idea is merging all those functionalities into a class that gets injected instead of having o inject each utility individually. What do you think?

[marciniwanicki]

Thanks @pepibumur for the very quick feedback! I was thinking about your approach, I've seen something similar some time ago, but I don't have any experience in using it. It's definitely more idiomatic and does not look like Java style programming. Some questions/thoughts:

• would it mean that the Context needs to implement (in extensions) all the methods of FileHandling, Logging, and of all other protocols we'd like to inject, and it would forward all the calls to the private properties implementing those individual protocols? there is a risk the protocols have conflicting methods.
• the usage looks very clean but accessing the properties would bring back self.-type feeling where we have "context" almost everywhere, one of the alternatives would be to unwrap the context

class Command {
  let fileHandler: FileHandling 
  let logger: Logging

  init(context: FileHandling && Logging) {
    fileHandler = context
    logger = context
  }
}

// otherwise we would need to use
context.logger.log("Log this line")
context.fileHandler.exists(path) 

• once we have the context object, the creation of an instance will lose visibility of its dependencies

return Command(context: context) // not clear what it depends on

Very similar alternative I've seen a while ago is to have a provider for each dependency:

protocol FileHandlerProvider { 
   var fileHandler: { get }
}

protocol LoggerProvider { 
   var logger: { get }
}

class Context {}
extension Context: FileHandlerProvider { 
   let fileHandler: FileHanding = FileHandler() // or computed
}
extension Context: LoggerProvider {
   let logger: Logging = Logger() 
}

class Command {

   let fileHandler: FileHanding
   let logger: Logging

   init(dependencies: FileHanderProvider && LoggerProvider) { 
      fileHandler = dependencies.fileHandler
      logger = dependencies.logger
   }
}

• This approach avoids conflicts but requires to write so many protocols.
• I think the technique is quite good to share stateless objects that normally would become singletons ( ugh 😬 ), but I think it does not play well with objects which keep some state thus need to be create separately for each parent. In that cases both stateless (singleton) and stateful objects looks the same at the creation time:

let command = Command(dependencies: dependencies) 
// and instance the Command class
dependency.fileHandler // not sure if I will get one instance only for myself or I share it with others

• I think it's quite nice if all the classes are DI agnostic so it's easy to switch from one solution to another (hm.. well I guess it's not true for languages with annotations). Using context or dependencies reveals quite a bit of the underlying details. Also, passing the container (i.e. Swinject container) to many classes, in general, is an anti-pattern (in my eyes - I should probably check what other people in the internet think about it 😅)

Regarding the factory, I know - it does look odd. In terms of terminology (again, I might be wrong here😛 ) - as I understand, factory simplifies creation of objects (from the caller perspective) and allows to easily replace the implementation it returns in compile or in run-time (if needed). If we decide that our logger now should use os_log, it's going to be a single line change to make the project use new implementation (i.e. OSLogger extending Logging). We could probably leverage the abstraction of the factory main product (Generating) when we generate different types of projects, like buck or bazel. Currently our dependency tree is pretty static (known at compile-time).
I feel that the factory-like class would be also nice even if we use an open-source dependency container. That would hide the existence of the 3rd party and wouldn't require the client module to understand its API/complexity. Also, it might be a good place for the DI initialization (assemblies).

[ollieatkinson]

Thanks for raising this @marciniwanicki.

Dependency injection is a toughie, and Swift doesn't make it very easy and/or elegant out of the box. The current system we have does mean you need to pass objects though which sometimes have no relevance in the middle layer - which can cause confusion and potential misuse.

But... I don't think it's bad, I really like the use of the protocols and knowing at initialisation the requirements. I've not found it to be much of a problem so far.

One of the problems I can see with creating a Context object is that every class will need to have it's own context, or they will all have to share a context - the former doesn't really gain much as we still have to support all of the different types and will need to convert between the contexts and the latter isn't any different from accessing the objects through a singleton (which is suggested not such a bad idea for this kind of problem).

However, I've often thought about the idea of a concentric state model, which is an immutable object that is copied and appended to when passed down the stack. The main idea being that you cannot modify anything, only add to it or pass it along as it is.

I haven't thought too much yet about how the code might look, but you might want to have some API similar to the following:

struct Dependencies: Equatable, Codable {
    
    func get<T>(_ dependency: T.Type) -> T
    func appending<T>(_ dependency: T) -> Self

}

It's a little ramble, so apologise for that - it might be worth setting up a call on slack to go over a few of your options and talk through them.

[kwridan]

a small victim of a missing dependency being passed around #389 😛

[pepicrft]

Would you like to champion this effort @marciniwanicki? With more components being added, having to pass the dependencies down is becoming significantly cumbersome. This would be a great addition to Tuist.

[marciniwanicki]

Sure 👍 I am so happy you're enthusiastic to move it forward! :) I will create a doc with all the options (including pros and cons of each) so we can have a good base to make some final decisions.

[marciniwanicki]

Sorry for keeping this ticket on hold, had ultra busy time at work - but it's all done and I should be a bit more responsive now 🤗. I was trying to research some alternatives but I still think "factory" or "builder" type class per module would be the best option.
I might be biased, usually work on relatively big projects where explicit dependencies and consistency in the way they are set up is the key to make the code flexible and easily understandable by other contributors.

I've pushed an extended version of the "factory", levering Swinject (https://github.com/bloomberg/tuist/tree/spike/MI/351-dependency-inject-via-swinject/Sources/TuistGenerator). Code-wise, the assembly (https://github.com/bloomberg/tuist/blob/spike/MI/351-dependency-inject-via-swinject/Sources/TuistGenerator/TuistGeneratorAssembly.swift) does not look much better than manual factory class (https://github.com/bloomberg/tuist/blob/spike/MI/351-dependency-inject-via-swinject/Sources/TuistGenerator/TuistGeneratorFactory.swift) but it's generally nicer to use Swinject as it reports circular dependencies in a very clean way.

In principal, each module would define each own assembly, TuistKit would set up

        c.register(Systeming.self) { _ in System() }.inObjectScope(.container)
        c.register(Printing.self) { _ in Printer() }.inObjectScope(.container)
        c.register(FileHandling.self) { _ in FileHandler() }.inObjectScope(.container)
// and other common/shared instances

so other assemblies wouldn't need to create them, just resolve them where needed as assemblies can pull dependencies from each other.

I think the overhead of writing and maintaining the assemblies is quite small compare to the benefits:

• pure classes
• each class can use only what it needs
• it very clear what classes using FileSystem, what classes can print something to the stdout, or which classes take too many responsibilities (long list of dependencies) just by looking at their constructor.
• a single, simple way to create new classes (no init in the code base, except data classes/structs - logic classes are only constructed in assemblies)
• no shared instances that can accidentally leak some state or make the implementation less flexible
• Injection helps to ease the process of writing integration tests i.e. using all the real classes except the nodes which are using I/O. That helps to write integration tests which are as fast as unit tests.

I think there are 2 potential problems with shared instances:

• They are either final - which prevents us from mocking them
• Or var, which is an overexposure, and every single line in the code is able to replace it.

I see the benefits and convenience of using such objects, they are great, especially in small projects, but I think once we have more people working on the codebase, and the project itself is growing🚀, people might want to use it in different environments/scopes, and that solution might not scale well.

If you'd be happy to give Swinject a shot, I can try to convert the whole project over the weekend (or next week).

[pepicrft]

Hey Marcin.

Thanks for getting back to this issue. I think the solution that you proposed is very explicit and provides very interesting features like being able to scope the dependencies that are being injected. However, I think it introduces unnecessary overhead for what we need in Tuist.

This option is definitively worth considering at some point, but given the amount of utilities that we are moving around (just a few), I don't think it's the overhead.

I made a proposal on this PR that uses shared instances, and provides helpers that are very handy when writing tests.