Resolvers are using the global container instead of the registered scoped container

[rfgamaral]

Describe the bug

I am using fastify-gql for this GraphQL API I'm building with type-graphql and I have this requirement where I need to manually create/initialize my services per request context. These services are meant to be injected/used by my resolvers and I'm trying to glue the whole thing together with typedi.

I believe I have followed the scoped containers documentation correctly but my resolvers, for some reason, are not using my custom container instance. Instead, my resolvers are getting my services from the global container, which I'm not using.

To Reproduce

1. Clone https://github.com/rfgamaral/typegraphql-typedi-sample;

   The original source code example (if the repo is deleted)

   import 'reflect-metadata'; import Fastify, { FastifyRequest } from 'fastify'; import cors from 'fastify-cors'; import GQL from 'fastify-gql'; import helmet from 'fastify-helmet'; import { Field, FieldResolver, ObjectType, Query, Resolver, ResolverData, ResolverInterface, buildSchemaSync, } from 'type-graphql'; import { Container, ContainerInstance } from 'typedi'; type GraphQLContext = { container: ContainerInstance; }; @ObjectType() class Model { @Field() date!: Date; } class ModelService { initialize(): void { console.log('ModelService:initialize()'); } } @Resolver(() => Model) class ModelResolver implements ResolverInterface { constructor(container: ContainerInstance) { console.log('[2] CONTAINER', container); console.log('[2] CONTAINER', container.has('model-service')); } @Query(() => Model) async model(): Promise { return new Model(); } @FieldResolver() async date(): Promise { return new Date(); } } const fastify = Fastify(); fastify.register(cors); fastify.register(helmet); fastify.addHook('onResponse', (request, _, done) => { Container.reset(request.id); done(); }); fastify.register(GQL, { path: '/graphql', context: (request: FastifyRequest): GraphQLContext => { const container = Container.of(request.id); const modelService = new ModelService(); modelService.initialize(); container.set('model-service', modelService); console.log('[1] CONTAINER', container); console.log('[1] CONTAINER', container.has('model-service')); return { container, }; }, schema: buildSchemaSync({ container: ({ context }: ResolverData) => { return context.container; }, resolvers: [ModelResolver], }), jit: 1, }); fastify.listen(3000, err => { if (err) { fastify.log.error(err); process.exit(1); } });

2. Install dependencies and run server with yarn start;

3. Make a GraphQL request to http://localhost:3000/graphql with { model { date } };

4. Look at the console and observe the logged output:

[1] CONTAINER ContainerInstance {
  services: [ { id: 'model-service', value: ModelService {} } ],
  id: 1
}
[1] CONTAINER true
[2] CONTAINER ContainerInstance {
  services: [],
  id: ContainerInstance {
    services: [ [Object], [Object], [Object] ],
    id: 1
  }
}
[2] CONTAINER false

The container instance used by ModelResolver is the global one, instead of the custom container instance created with a manually initialized ModelService. You can also observe that the container instance used by ModelResolver returned false when trying to check for the existence of model-service.

Expected behavior

The container instance used by ModelResolver should be the custom container instance registered to be used by Type-GraphQL.

Enviorment (please complete the following information):

• Windows 10 (WSL)
• Node v12.12.0
• Yarn v1.21.1
• TypeScript v3.8.3

[MichalLytek]

ContainerInstance is not the instance of the container, it's a global container instance.

As you are providing Container.of to context, you should just do:

class ModelResolver implements ResolverInterface<Model> {
  constructor(
    @Inject('model-service') modelService: ModelService,
  ) {}
}

[rfgamaral]

ContainerInstance is not the instance of the container, it's a global container instance.

But shouldn't it be?

I mean, if I'm registering a custom container instance and telling type-graphql to use it, why is the default mechanism to use the global container instance instead of the custom one we are telling the engine to use?

[MichalLytek]

But shouldn't it be?

Please open an issue on type-di repository about that.

ContainerInstance is a class that is used (new ())by Container.of and other methods, so providing it as a constructor parameter type won't automatically magically bind it with the one from the context.

why is the default mechanism to use the global container instance

You have not provided Container.set(ContainerInstance, myScopedContextContainer) so it inject the one from global container.

[rfgamaral]

But shouldn't it be?

Please open an issue on type-di repository about that.

Based on the documentation for registering a 3rd party IoC container I saw the following examples:

import { Container } from "typedi";
const schema = await buildSchema({
  container: Container, // global container
});

import { Container } from "typedi";
const schema = await buildSchema({
  container: (({ context }: ResolverData<TContext>) => context.container); // scoped container
});

Based on the above I was under the impression that we would have the flexibility to tell TypeGraphQL exactly what to use through the server lifecycle, either the global container or a scoped one. In other words, I believed that this was TypeGraphQL responsibility and not typedi.

ContainerInstance is a class that is used (new ())by Container.of and other methods, so providing it as a constructor parameter type won't automatically magically bind it with the one from the context.

I never expected my scoped contained to be automatically binded based on the parameter type, I expected it based on the registration of the scoped container in the buildSchema call as in the example above.

You have not provided Container.set(ContainerInstance, myScopedContextContainer) so it inject the one from global container.

This doesn't feel like a good idea and could be the source for future problems.

---

Anyway, now I'm a bit confused about the purpose of the container property in the buildSchema options... Can you clarify for me when/how this property is used and how can I access the scoped container in my resolvers?

[MichalLytek]

The "purpose of the container property in the buildSchema" is that you can use NestJS container, Inversify, etc. TypeGraphQL will call their .get() method to retrive the instance of resolver class. And that's all.

You can provide a static value (global container) or provide a method that would be called every time when resolver class instance is needed.

This way you can create new container based on the request id, put it inside the context, and then by using the function you can retrive it from context and pass to TypeGraphQL.

TypeGraphQL has nothing to do with resolving services depdencedies or other magic.

[rfgamaral]

This way you can create new container based on the request id, put it inside the context, and then by using the function you can retrive it from context and pass to TypeGraphQL.

This is where I'm most confused because I'm able to do all this without setting the container in buildSchema at all. For instance, take this change as an example:

• https://github.com/rfgamaral/typegraphql-typedi-sample/compare/without-container

  • The original code change (if the repo is deleted)

  

Without setting the container I'm still able to get the scoped container from the context and access my custom initialized services. Am I missing anything by not setting the container?

[MichalLytek]

Am I missing anything by not setting the container?

Yes, this means TypeGraphQL gonna fallback to default container and not use your container to resolve the dependecies.

And it means that when you perform a GraphQL request, TypeGraphQL will use your resolver classes without any dependencies (new MyResolver();).

[rfgamaral]

Yes, this means TypeGraphQL gonna fallback to default container and not use your container to resolve the dependecies.

What dependencies? I'm resolving them manually by getting the container from context and calling get myself...

And it means that when you perform a GraphQL request, TypeGraphQL will use your resolver classes without any dependencies (new MyResolver();).

Again, what dependencies are you referring to? By creating my own scoped container and place it in the context, I'm forced to retrieve the container from that context and call get myself to get the dependencies I need. So I don't understand where TypeGraphQL will fail...

---

Maybe I'm not making myself clear enough and I apologize for that, English is not my main language, but let me try to explain exactly what I'm trying to do by enumerating some points:

• I need to have a scoped context per context and since I'm using fastify-gql I can create the scoped container in the context function which is executed once per request received (this step is one as you can see in the code above);
• I need to manually create my services, call some custom to initialize functions and place the services instances in the scoped container (this is step is also done as you can see in the code above);
• I need to inject my manually created services inside my resolvers by constructor injection or the @Inject decorator (I prefer this one) but both these methods fallback to the global container and ignore my scoped one completely. I haven't yet found a way to do this point. Do you have a suggestion?

[rfgamaral]

I think I've figured it out, here's the code I have ended up with:

import 'reflect-metadata';

import Fastify, { FastifyRequest } from 'fastify';
import cors from 'fastify-cors';
import GQL from 'fastify-gql';
import helmet from 'fastify-helmet';
import {
    Field,
    FieldResolver,
    ObjectType,
    Query,
    Resolver,
    ResolverData,
    ResolverInterface,
    buildSchemaSync,
} from 'type-graphql';
import { Container, ContainerInstance, Inject } from 'typedi';

type GraphQLContext = {
    container: ContainerInstance;
};

@ObjectType()
class Model {
    @Field()
    date!: Date;
}

class ModelService {
    id = 0;

    initialize(): void {
        console.log('ModelService:initialize()');

        this.id = Math.floor(Math.random() * 1000);
    }
}

@Resolver(() => Model)
class ModelResolver implements ResolverInterface<Model> {
    @Inject()
    modelService1!: ModelService;

    modelService2: ModelService;

    constructor(@Inject(() => ModelService) modelService: ModelService) {
        this.modelService2 = modelService;
    }

    @Query(() => Model)
    async model(@Inject(() => ModelService) modelService: ModelService): Promise<Model> {
        console.log('[1]', this.modelService1);
        console.log('[2]', this.modelService2);
        console.log('[3]', modelService);

        return new Model();
    }

    @FieldResolver()
    async date(): Promise<Date> {
        return new Date();
    }
}

const fastify = Fastify();

fastify.register(cors);
fastify.register(helmet);

fastify.addHook('onResponse', (request, _, done) => {
    Container.reset(request.id);

    done();
});

fastify.register(GQL, {
    path: '/graphql',
    context: (request: FastifyRequest): GraphQLContext => {
        const container = Container.of(request.id);

        const modelService = new ModelService();
        modelService.initialize();

        container.set(ModelService, modelService);

        return {
            container,
        };
    },
    schema: buildSchemaSync({
        container: ({ context }: ResolverData<GraphQLContext>) => {
            return context.container;
        },
        resolvers: [ModelResolver],
    }),
    jit: 1,
});

fastify.listen(3000, err => {
    if (err) {
        fastify.log.error(err);
        process.exit(1);
    }
});

And here are my assumptions after all this, please correct me of anything I got wrong or if you any other suggestion:

1. With fastify-gql, the context function will be called once per HTTP request received and this is where I'm creating my scoped container per request and any custom services I need to initialize manually; Since I'm manually creating/initializing my services and placing them in my scoped container, there's no need to use @Service() on my injectable services;
2. To inject my custom services in my resolvers I need to a) use @Inject() if it's a class field; b) use @Inject(() => ModelService) if it's a constructor parameter (this syntax also works for a class field); However, neither @Inject() nor @Inject(() => ModelService) worked as method parameter, why?
3. If I don't specify the container property when calling buildSchema (or buildSchemaSync) with the same scoped container created with the context function (as mentioned in point 1), the injection mentioned in point 2 will not work and the console.log calls will print undefined; I understand now why this is needed;
4. I'm sure the injected instances are the ones I'm expecting because the console.log calls will print the ModelService instance with a random number per request (which is only set if ModelService.initialize is called, otherwise it should've been 0) and always the same number in all console.log calls.
5. As a conclusion, everything seems to be working as expected and with the minimal required configuration to glue everything together. Correct @MichalLytek?

[MichalLytek]

However, neither @Inject() nor @Inject(() => ModelService) worked as method parameter, why?

Because methods are called directly by TypeGraphQL - injecting args, context, info or handling customParamDecorator

Correct @MichalLytek?

Looks like so 😉 So the only difference between the examples and your code is that you don't use @Service decorator with scope option but manually putting them in a scoped container.

[rfgamaral]

However, neither @Inject() nor @Inject(() => ModelService) worked as method parameter, why?

Because methods are called directly by TypeGraphQL - injecting args, context, info or handling customParamDecorator

So, in other words, the @Inject() was never executed and I tried to access a variable that was never set. It makes sense now.

So the only difference between the examples and your code is that you don't use @Service decorator with scope option but manually putting them in a scoped container.

I tried using @Service and it worked just the same. But since I'm manually creating them and putting them in a scoped container, it doesn't make much sense to me to keep decorating the classes with @Service, that's why I removed it.

---

The thing is, I'm almost certain that I tried this approach before and it didn't work, I was always getting undefined dependencies. I was only able to get it working as I wanted after creating the minimal example but even now I can't figure out what the hell did I do differently this time, the code looks exactly the same to my first approaches... 🤷‍♂️

I'm just glad I finally got it working. Thank you for bearing with me :)

[desmap]

@MichalLytek I also trying to grok typedi for few days and it feels buggy/inconsistent + subpar docs + the repo doesn't seem to maintained anymore. I lost some time and think that if there are good alternatives out, is it an option to remove typedi from you docs and example and introduce another maintained DI lib?

[MichalLytek]

@desmap TypeGraphQL is really DI container agnostic and it can work with whatever container you want.

The reason why I use it in examples is that it's really simple to configure, comparing to e.g. InversifyJS boilerplate.

The examples are just simple examples, not a real-world starter repo you should use 😉

If you know some maintained DI lib that has the same sets of features as TypeDI and it's easy to use, please show it to me - maybe it would be a good replacement.

[desmap]

@MichalLytek thanks for your quick reply and I understand!

I checked them all and my feeling is that tysringe and injection-js (the one ripped out of Angular) are the front runners with tysringe being more intuitive and more simple. I don't have a lot experience with DI, so please check them out as well. I also posted a thread on Reddit and one person suggested I should write my own DI :) https://www.reddit.com/r/typescript/comments/fo1equ/typedi_cheat_sheet/

In any case, I would remove typedi from all your docs. Even if it's simple to set up, which I agree to, typedis naming scheme is utterly wrong + super confusing + it has weird docs. Paired with a repo in limbo state and an unresponsive maintainer who constantly feels betrayed by open source (I mean just check typeorm). This also negatively reflects on your repo.

Since DI is important for any kind of CRUD (just think of reusing the DB connection w/o creating new instances) it would be great to have, like you have now, few examples with one or two sane DI libs.

It's just that people who want to setup type-graphql seriously and build their app around it, want and need to understand DI properly and I just wasted days on typedi because I assumed it's the reference solution and documented and better stay with the defaults.

[MichalLytek]

@Inject() injects Container into decorated class properties,

Why do you want to inject container into a class property?

I don't have a lot experience with DI,

Ok, maybe the reason is that you need to understand the dependency inversions principle first, then manual constructor injection and on top of them - service locator and dependency injection framework.

[desmap]

Why do you want to inject container into a class property?

I don't want to. I just wanted to understand typedis API which has an @Inject decorator function.

you don't understand the dependency inversions principle

Maybe, but after days spending on this, I got now a basic understanding on DI and find typedi still odd. I think you had a longer debate with the maintainer re the strange naming scheme yourself. Btw a subpar naming schemes hurts long-term code maintainability.

And btw, even if I have absolutely no clue, why not just use a more intuitive lib in your docs for the not so advanced ones?

[MichalLytek]

I just wanted to understand typedis API which has an @Inject decorator function.

Given your class contract:

class MyService {
  constructor(
    private myDependency: MyDependency, 
  ) {}
}

You can choose to:

a) place @Service on top of the MyService class to "inform" TypeDI about the constructor parameters, so it can resolve the dependencies

@Service()
class MyService {
  // ...
}

b) use @Inject to manually provide the information what you want to inject - it can be the class (auto bind):

  constructor(
    @Inject(MyDependency)
    private myDependency: MyDependency, 
  ) {}

or your value if you want to control providing the values:

  constructor(
    @Inject('my-dependency')
    private myDependency: MyDependency, 
  ) {}

but then you need to register that in container:

Container.set('my-dependency', new MyDependency(myOtherDepValue));

So @Inject is a good name that is used in angular in Nest.

My only issue was about @Service which might be misleading - @Injectable is also not a precise name but at least it has some info about injecting in the name.

why not just use a more intuitive lib in your docs for the not so advanced ones?

https://github.com/microsoft/tsyringe looks nice, although I don't like lowercase decorator names. Maybe in vNext I gonna use that as a reference example 😉

[rfgamaral]

I've been debating the same thing lately and I've settled with typedi. Yeah, the project is not as maintained as one would like but this is open-source, take it or leave it.

I'm still using typedi on my project and comparing it to tsyringe, well, there's just something odd for me in tsyringe and I also dislike the lowercased decorator names 😅 Another thing I don't like is a feature they are explicitly saying they won't be adding (property injection).

[desmap]

@MichalLytek still struggling with @Service(). I assumed that I can decorate classes with @Service() in order to provide their singleton instances as services to other classes, e.g. a class which just connects a database only once and provides it (or provide this service) to other classes with its connection as a singleton service. Besides, the named @Service(<name>) does exactly this.

The plain @Service() as you write, just informs TypeDI that stuff get injected into this class which is the opposite. So this doesn't mean that I want to provide the @Service()-decorated class as service to other classes. Maybe I just don't get it or the use case but I just want an easy way to provide singletons (like DB connections) to freshly initiated class instance per request (like user.save())

However, I think...

Maybe in vNext I gonna use that as a reference example

this is great, just to have some options and I agree I am also not super happy about the lower cased names, CC @rfgamaral

[desmap]

I think I slowly get what @pleerock actually meant with @Service: service the decorated class with dependencies from Container... and not provide the decorated class as a service (what I thought the entire time)