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

Will this work with Typegraphql? #158

Open
algoflows opened this issue Mar 21, 2024 · 3 comments
Open

Will this work with Typegraphql? #158

algoflows opened this issue Mar 21, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request further info needed The subject of the issue isn't clear question Further information is requested

Comments

@algoflows
Copy link

algoflows commented Mar 21, 2024

Will this work with Typegraphql and Bun? I will try it tonight and see how far I get. I am also new to DI so not sure about the library or technology as their isn't much documentation on it here.

So yes, big question will this work with Typegraphql?! Thanks for all the awesome work.

Update

It didn't work unfortunately, would love for it to though!

I tried to pass the default {Container} to the graphql builder both Apollo and Graphql Yoga builders and it errors as they expect get() and Typedi does match their interface.

Theres a quick win if you can support Apollo and Graphql yoga you'd have quite a few users of this library as many use Typegraphql in production and are searching for DI libraries that are still being maintained.

Here's a thread on it:

https://www.reddit.com/r/typescript/comments/fo1equ/typedi_cheat_sheet/

Typedi++ would need a small wrapper to match the graphql builder interfaces which would be big win for us as we could easily just switch to using Type++ :))

@algoflows algoflows added the question Further information is requested label Mar 21, 2024
@freshgum-bubbles
Copy link
Owner

Hello. I'm not sure what TypegraphQL is.

Regarding Bun support, this library should support it. If you could elaborate upon the issues raised (in a separate ticket, with logs and a repro.) I'd be more than happy to check that out.

If it helps a lot of people, I'd be happy to include the proposed wrapper as a contrib package (PRs are accepted, of course :P).

@algoflows
Copy link
Author

algoflows commented Mar 22, 2024

import { buildSchema } from "type-graphql";

// IOC container
import { Container } from "typedi";
import { SampleResolver } from "./resolvers";

// Build TypeGraphQL executable schema
const schema = await buildSchema({
  // Array of resolvers
  resolvers: [SampleResolver],
  // Registry 3rd party IOC container
  container: Container,
});

Any chance you could support the 3rd party IOC container? It would mean matching the container interface that BuildSchema expects....

I don't know enough about Containers or DI to build libraries, but we rely on them at work and Typedi's maintanance death has us looking for other solutions and you've built a library that could support the many develops building with Typegraphql and Apollo Ecosystem.

The Reddit thread posted earlier said it would be quite trivial for someone who knows what they are doing, as you built the library I would think this would be quite easy for you.

https://typegraphql.com/docs/dependency-injection.html

Thanks!

@freshgum-bubbles
Copy link
Owner

Hello. The late timing of my response is, in part, due to personal circumstances.

Any chance you could support the 3rd party IOC container? It would mean matching the container interface that BuildSchema expects....

I'm still trying to understand the heart of the issue here. I've had a look at the TypegraphQL documentation and it gives the following example code:

import { buildSchema } from "type-graphql";
// IOC container
import { Container } from "typedi";
import { SampleResolver } from "./resolvers";

// Build TypeGraphQL executable schema
const schema = await buildSchema({
  // Array of resolvers
  resolvers: [SampleResolver],
  // Registry 3rd party IOC container
  container: Container,
});

This, of course, leads me to believe that it's compatible with TypeDI. This library is (mostly) compatible with the same, but some differences were made to improve the UX of the library. If this issue is simply predicated by the non-existence of a 1:1 compatibility layer with the legacy TypeDI module, I could be swayed by the inclusion of a compatibility layer in the form of a contributory package. So... is that what's required?

I'll take a look at the thread and try to get my bearings -- as said, I'm very happy to look at implementing this.

Bear in mind that I take quite a simplistic view of development when it comes to this library: the base container class will not, and will never, be designed to support thousands of edge cases -- these are typically done as external contributory packages, which can then be optionally linked in to enhance the behaviour of the library. This is preferred to keep the bundle size low.

@freshgum-bubbles freshgum-bubbles added enhancement New feature or request further info needed The subject of the issue isn't clear labels Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request further info needed The subject of the issue isn't clear question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants