DI icon indicating copy to clipboard operation
DI copied to clipboard

ISP complicated or violated with wessberg/DI

Open DScheglov opened this issue 4 years ago • 4 comments

Hey, guys,

the library uses interface names as tokens and it complicates following to Interface Segregation Princinple.

In example:

interface IInfoLogger {
  info(message: string): void;
}

interface IWarnLogger {
  warn(message: string): void;
}

interface IErrorLogger {
  error(error: Error): void;
}

All of these interfaces belongs to the clients of some logger, for example:

class MyService {
  consturctor(private readonly logger: IInfoLogger) {
    logger.info(...);
  }
}
class MyOtherService {
  consturctor(private readonly logger: IWarnLogger) {
    logger.warn(...);
  }
}
class MyThirdService {
  consturctor(private readonly logger: IErrorLogger) {
    logger.error(new Error('....'));
  }
}

and finally I have the implementation

export interface ILogger extends IInfoLogger, IWarnLogger, IErrorLogger {}

export default class ConsoleLogger implements ILogger {
 ....
}

So, with wessberg/DI I have to register the ConsoleLogger three times with different client owned interfaces, and it could be ok, but when we need to make ConsoleLogger a singleton shared between all clients, -- we can't do it with wessberg/DI, can we?

DScheglov avatar Apr 27 '21 11:04 DScheglov

Looks like you can do something like that

const consoleLogger = new ConsoleLogger(); // or another way to instantiate 
container.registerSingleton<IInfoLogger>(() => consoleLogger);
container.registerSingleton<IWarnLogger>(() => consoleLogger);
container.registerSingleton<IErrorLogger>(() => consoleLogger);

Serg046 avatar Aug 23 '22 10:08 Serg046

Oh! ) @Serg046 greate idea ...

But, what if some service have dependency like: IInfoLogger & IWarnLogger? We need to know all combinationes ...

DScheglov avatar Aug 23 '22 11:08 DScheglov

I am not a FE dev and not that familiar with ADT but something like that should work here too (so you just add one more registration)

const consoleLogger = new ConsoleLogger(); // or another way to instantiate 
container.registerSingleton<IInfoLogger>(() => consoleLogger);
container.registerSingleton<IWarnLogger>(() => consoleLogger);
container.registerSingleton<IErrorLogger>(() => consoleLogger);

type IInfoWarnLogger = IInfoLogger & IWarnLogger;
container.registerSingleton<IInfoWarnLogger>(() => consoleLogger);
//or even
container.registerSingleton<IInfoLogger & IWarnLogger>(() => consoleLogger);

You could even write your own transformer that would register all the interfaces and their combinations automatically

Serg046 avatar Aug 23 '22 11:08 Serg046

@Serg046 , I'm not a FE dev too. Perhaps you mean, you don't code with Typescript ...

Sure you can declare:

interface IInfoWarnLogger extends IInfoLogger, IWarnLogger {}
// or as you've proposed with type conjunction

But how many interfaces should we declare? How many times should we register the same factory/class? What if we need not a singleton, but we need to create a logger for each "request"? What about if we need a different instances implementing the same interface for different dependent services (service A needs Logger with log level "errors only", but service B needs Logger with log level "verbose")?

You could even write your own transformer that would register all the interfaces and their combinations automatically

  • actually I have own solution for the DI: https://github.com/DScheglov/true-di
  • I consider to use Interface/Types names in structured (non nominal) typing system is a bad idea because it kills the benefits from the structural typing and it provokes to write much more code then we actually need (the TS is enough criticised for verbosity, so why do we need to make it worse)
  • true-di uses dedicated tokens for components, so it is also not ideal, but it is two years in production and works well.

DScheglov avatar Aug 23 '22 11:08 DScheglov