vovaspace / brandi Goto Github PK

I have a singleton class that is being injected into other classes using the injected(...) syntax.
The class is called Exchange.
I need to add an async initializer to that class. (want to set up a websocket before the class is used)
I'm trying to do so by creating a Factory but I'm running into a roadblock. Can I still used injected() when using an async Factory?

I have tokens defined as follows:

    exchange: token<Exchange>('Exchange'),
    exchangeFactory: token<AsyncFactory<Exchange>>('Exchange Factory'),

and container bindings set up like this,

    .bind(TOKENS.exchange)
    .toInstance(ExchangeAlpaca)
    .inSingletonScope();

container
    .bind(TOKENS.exchangeFactory)
    .toFactory(async () => {
        let e = container.get(TOKENS.exchange);
        await e.init();
        return e;
    });

class SomeClass {
  constructor(private e: Exchange){}
}
injected(SomeClass, TOKENS.exchangeFactory.optional) // <- this doesn't work
injected(SomeClass, TOKENS.exchange.optional) // <- this works but factory doesn't get used.

I'm trying to implement this library and I'm having trouble getting an instance at runtime when the constructor requires a dependency. Constants and instances (that don't have dependencies) work fine. What am I doing wrong?!

Here is the stack it throws:

/Users/merlin/Development/app/server/node_modules/brandi/lib/brandi.js:416
    if (target.length === 0)
               ^

TypeError: Cannot read properties of undefined (reading 'length')
    at Container.getParameters (/Users/merlin/app/server/node_modules/brandi/lib/brandi.js:416:16)
    at Container.createInstance (/Users/merlin/app/server/node_modules/brandi/lib/brandi.js:397:29)
    at Container.resolveCache (/Users/merlin/app/server/node_modules/brandi/lib/brandi.js:392:27)
    at Container.resolveBinding (/Users/merlin/app/server/node_modules/brandi/lib/brandi.js:369:21)
    at Container.resolveToken (/Users/merlin/app/server/node_modules/brandi/lib/brandi.js:356:19)
    at Container.get (/Users/merlin/app/server/node_modules/brandi/lib/brandi.js:348:17)
    at Object.<anonymous> (/Users/merlin/app/server/dist/routes/assets.js:12:42)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Here is my implementation:

import { PrismaClient } from "@prisma/client";
import { token } from "brandi";
import { AssetRepository } from "../repositories";
import { TimingService } from "../services";

const TOKENS = {
    prismaClient: token<PrismaClient>("prismaClient"),
    assetRepository: token<AssetRepository>("assetRepository"),
    timingService: token<TimingService>("timingService")
}

const container = new Container();

container.bind(TOKENS.prismaClient).toConstant(new PrismaClient());
container.bind(TOKENS.assetRepository).toInstance(AssetRepository).inContainerScope();
container.bind(TOKENS.timingService).toInstance(TimingService).inSingletonScope();

My class

// this one breaks when I try to instantiate
export class AssetRepository {
    db: PrismaClient;

    constructor(prismaClient: PrismaClient) {
        this.db = prismaClient;
    }

// a bunch of other methods
}

injected(AssetRepository, TOKENS.prismaClient);

pulling it out of the container:

// this works
const db = container.get(TOKENS.prismaClient);

// breaks here
const assetRepository = container.get(TOKENS.assetRepository)

Would it be possible to rework things so that container.get returns a Promise and toFactory takes a factory that returns a Promise to the type?

Hey, @vovaspace

This line violates Dependcy Inversion Principle:

The ES-module with definition of ApiService class depends on infrastructure layer: your library brandi and on the module structue: TOKENS.

Just move this line to the composition root and everything becomes well again:

import { DependencyModule, injected } from 'brandi';
import { TOKENS } from '../tokens';
import { ApiService } from './ApiService';

export const apiModule = new DependencyModule();

apiModule.bind(TOKENS.apiKey).toConstant('#key9428');

injected(ApiService, TOKENS.apiKey); // << this code should be here
apiModule.bind(TOKENS.apiService).toInstance(ApiService).inTransientScope();

All dependency resolution rules should be placed in single place: composition root (see: Dependency Injection Principles, Practices, and Patterns by Steven van Deursen and Mark Seemann)

And if you are going to move it to the composition root, just add the inject parameter to the container methods: toFactory and toInstance:

import { DependencyModule } from 'brandi';
import { TOKENS } from '../tokens';
import { ApiService } from './ApiService';

export const apiModule = new DependencyModule();

apiModule.bind(TOKENS.apiKey).toConstant('#key9428');

apiModule
  .bind(TOKENS.apiService)
  .toInstance(ApiService, TOKENS.apiKey)
  .inTransientScope();

First of all, let me say that this is an awesome library and I'm enjoying using it very much.

This is a feature request. If I'm understanding the docs correctly, currently, if I want to inject a class with an array of things, I have to do something like this to get the array injectable:

class ArrayOfStrings extends Array<string> {}
container.bind(Tokens.TestString).toConstant('hello');
container.bind(Tokens.TestString2).toConstant('brandi.js');
container.bind(Tokens.TestArray).toInstance(ArrayOfStrings).inSingletonScope();
injected(ArrayOfStrings, Tokens.TestString, Tokens.TestString2);
injected(SomeOtherClass, Tokens.TestArray);

This gets the job done, but it would be nice if there were a smoother mechanism. In particular, this line is kind of ugly:

injected(ArrayOfStrings, Tokens.TestString, Tokens.TestString2);

I don't want to be too specific about how that gets accomplished except to say that specifying the precise tokens of the injected array items is maybe less slick than marking them all somehow and then providing the marker to injected or some function like it.

Injected not working when disabled strictNullChecks. Because injected expect optional token.

Example:
https://www.typescriptlang.org/play?strictNullChecks=false#code/JYWwDg9gTgLgBAbzsAdgKwKYGMYYCYA0cMEA1hinAL5wBmUEIcA5AEZQCGKewzA3AFgAUMKwAbDgGdJcAIKIqw0ROlwAQomFw4WCCkkwoAVxzQAFGCjAAbh1xwOALjkBKBIqEfReg3AAqZBQyALyaQtpOxIEoADyyAHxmzBzMLgRacKzOJOSxaolsqemeSiJCqJg4+GZqRAG5kgB0HC6CZUA

import { injected, token } from 'brandi';

class A {}

class B {
  constructor(private a: A){}
}

const Tokens = {
  a: token<A>('a'),
  b: token<B>('b'),
}

injected(B, Tokens.a);

Error:

Argument of type 'Token<A>' is not assignable to parameter of type 'OptionalToken<A>'.
  Types of property '__o' are incompatible.
    Type 'false' is not assignable to type 'true'.

Current example of factories with arguments (and the related tests) seems to only allow for intializer injection into an already instantiated object, instead of allowing for a factory method that returns a new instance. This means it is impossible to inject the factory arguments as constructor arguments on the returned instance. I'm attempting to inject into a class who's parent I do not control, and need to inject dynamically created arguments into the constructor.

If there's a way to do this, can we update the documentation to reflect the method?

example:

import { token, Container, Factory } from 'brandi';

class OtherThing { } 

class UntouchableParent {
  protected dep: OtherThing;
  
  public constructor(dep: OtherThing) {
    this.dep = dep;
  }
}

class MyChild extends UntouchableParent {
  //...
}

const TOKENS = {
  myChildFactory: token<Factory<MyChild, [dep: OtherThing]>>('Factory<MyChild>')
};

const container = new Container();

container.bind(TOKENS.myChildFactory)
  .toFactory(MyChild, (dep: OtherThing): MyChild => new MyChild(dep));


const factoryInstance = container.get(TOKENS.myChildFactory);

const childInstance = factoryInstance(new OtherThing());

Hello!

I saw a library called ditox which has almost same interface like your library do and several questions emerged:

1. Isn't .inSomeScope() more error prone then ditox's {scope:"some"} argument? I sometimes forgive to add these scope calls and only remember that when see warnings in console
2. Your approach for declaring injection gives people ability to declare it in place of Class declaration thus might lead to mix up DI and services. (this is already mentioned in #28). Ditox's approach with generating factories instead leans to group all DI-related declarations together.
3. Your tag based conditional injection is a great idea! Do you think factory based instance generation is a little bit more flexible?

I wrote this issue not to clash you and @mnasyrov but to know your position on these design decisions. I already use your library in some projects and it might be better for others who find your or ditox to know which one fits their case best.

P.S. @mnasyrov @vovaspace don't you think it'll be great to have links to each other's projects to increase awareness of options?

There some issues with install package with react 18 and Functional Component with children prop is no longer supported.

We use [email protected] in our project and it worked well with [email protected]. But recently we had to update TypeScript version up to 4.9.5 and now we are facing with the following compilation error:

node_modules/.pnpm/[email protected]/node_modules/brandi/lib/typings/types.d.ts:5:70 - error TS2344: Type 'T' does not satisfy the constraint 'object'.

5 export declare type UnknownCreator<T = unknown> = UnknownConstructor | UnknownFunction;
~

node_modules/.pnpm/[email protected]/node_modules/brandi/lib/typings/types.d.ts:5:36
5 export declare type UnknownCreator<T = unknown> = UnknownConstructor | UnknownFunction;
~~~~~~~~~~~
This type parameter might need an extends object constraint.

Could you fix it?

Hello,

With the following code:

myContainer.get(myToken)

is there a useful way in VSCode to navigate directly to the implementation of myToken? I think to something similar to Ctrl + left mouse click on the myToken word, but it navigates to its definition with the token() function. I would like to navigate to the implementation associated to this token with the bind() function.

Currently when defining arguments using injected, one has to pass in the constructor as a first argument. This kinda removes a lot of the flexibility a DI container can give you. Let's say we want to do the following:

const TOKENS = {
  readCredentials: token<string>("readCredentials"),
  writeCredentials: token<string>("writeCredentials"),
  readConnection: token<DatabaseConnection>("readConnection"),
  writeConnection: token<DatabaseConnection>("writeConnection"),
  listTodosUseCase: token<ListTodosUseCase>("listTodosUseCase"),
  saveTodoUseCase: token<SaveTodoUseCase>("saveTodoUseCase"),
};

class DatabaseConnection {
  constructor(private credentials: string) {}
}

class ListTodosUseCase {
  constructor(private connection: DatabaseConnection) {}
}

class SaveTodoUseCase {
  constructor(private connection: DatabaseConnection) {}
}

container.bind(TOKENS.readCredentials).toConstant(config.db.credentials.read);
container.bind(TOKENS.writeCredentials).toConstant(config.db.credentials.write);
container.bind(TOKENS.readConnection).toInstance(DatabaseConnection);
container.bind(TOKENS.writeConnection).toInstance(DatabaseConnection);

/**
 * Perfect so far, we can give the different use cases the appropriate connections.
 */
injected(ListTodosUseCase, TOKENS.readConnection);
injected(SaveTodoUseCase, TOKENS.writeConnection);

/**
 * Here we run into an issue, we can't inject different connection strings into the same constructor.
 */
injected(DatabaseConnection, TOKENS.readCredentials);
injected(DatabaseConnection, TOKENS.writeCredentials);

You could say well, just create a read and write connection class and let them implement the same interface. But this defeats the purpose. The implementation of these classes will be the same, resulting in code duplication.

A great thing about some DI containers is that they give you flexibility to have the same class with different arguments injected, registered under different keys. (that's why the injected function should probably also not be in the same file as the class definition, this removes that flexibility).

I first thought conditional bindings with tags would solve this, but they are also bound directly to the constructor. That just moves the issue one level up.

My proposed solution is as follows, let's define the injected arguments per token instead of per constructor:

injected(TOKENS.listTodosUseCase, TOKENS.readConnection);
injected(TOKENS.saveTodoUseCase, TOKENS.writeConnection);

injected(TOKENS.readConnection, TOKENS.readCredentials);
injected(TOKENS.writeConnection, TOKENS.writeCredentials);

I can imagine, having the injected function as a separate function is then not even necessary anymore. We could do something like:

container.bind(TOKENS.readCredentials).toConstant(config.db.credentials.read);
container.bind(TOKENS.writeCredentials).toConstant(config.db.credentials.write);

container
  .bind(TOKENS.readConnection)
  .toInstance(DatabaseConnection);
  .withArguments(TOKENS.readCredentials);

container
  .bind(TOKENS.writeConnection)
  .toInstance(DatabaseConnection)
  .withArguments(TOKENS.writeCredentials);

I think this would greatly improve the flexibility of this library. Let me know what you think!

I'm interested in using a pattern like this for injected services:

import { injected, token } from 'brandi';

class MyService {
  static token = token<MyService>('MyService');

  // ...
}

injected(MyService, /* ... */);

so that token mappings are co-located with their relevant service implementations. But, there could be an issue if I introduce circular dependencies... for example:

// ./name.service.ts
import { injected, token } from 'brandi';
import { Logger } from './logger.service';

class NameService {
  static token = token<NameService>('NameService');

  constructor(private readonly log: Logger) {}

  async getName() {
    this.log.debug('called NameService.getName()');
    return 'Sally';
  }
}

injected(NameService, Logger.token);

// ./logger.service.ts
import { injected, token } from 'brandi';
import { NameService } from './name.service';

class Logger {
  static token = token<Logger>('Logger');

  constructor(private readonly nameSvc: NameService) {}

  async sayHello() {
    const name = await this.nameSvc.getName();
    console.log(`Hello, my name is ${name}!`);
  }

  debug(message: string) {
    console.log('[debug]', message);
  }
}

injected(Logger, NameService.token);

I think this could be resolved by allowing tokens to be provided as functions, like:

injected(MyService, () => MyOtherService.token, ...);

Then the token could be fully resolved when container.get() is called.