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.

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

ISP complicated or violated with wessberg/DI #4

Open
DScheglov opened this issue Apr 27, 2021 · 4 comments
Open

ISP complicated or violated with wessberg/DI #4

DScheglov opened this issue Apr 27, 2021 · 4 comments

Comments

@DScheglov
Copy link

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?

@Serg046
Copy link

Serg046 commented Aug 23, 2022

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);

@DScheglov
Copy link
Author

Oh! )
@Serg046 greate idea ...

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

@Serg046
Copy link

Serg046 commented Aug 23, 2022

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

@DScheglov
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants