MichalLytek / type-graphql

Create GraphQL schema and resolvers with TypeScript, using classes and decorators!
https://typegraphql.com
MIT License
8.03k stars 675 forks source link

TypeORM integration plugin #44

Open MichalLytek opened 6 years ago

MichalLytek commented 6 years ago

I wonder whether TypeGraphQL should stay as the uniwersal framework that works with many libs or to be more integrated with the libs that the most of the devs would use, like TypeORM.

I am thinking about the automatic relations resolving feature that could use lazy relation wrapper mechanizm or typeorm-loader for performance purposes.

The best way would be to create plugin system and create @typegraphql/typeorm helper package that would provide better integration. I have to investigate this way if it's possible.

Any ideas, guys ❓

j commented 6 years ago

I'd suggest keeping this out of this library and somehow (if custom decorator support,or similar, is made) make it so that 3rd parties can connect two different metadata libraries together. Perhaps just expose the internal API to define metadata? I think TypeORM + type-graphql is clean as is just having two decorators on fields. Personally, I'm not using TypeORM b/c I prefer vanilla mongo using the cursor map() function to map to models.

MichalLytek commented 6 years ago

With #14 and #45 it would be really easy to add another validation library or create a relation-resolving extension for sequelize.

I would have to think about exposing MetadataStorage as it's now quite low-level and internal. You think about something like generating schema from TypeORM @Column decorator or just an access to the storage to e.g. get a list of field of object type returned by quer?

capaj commented 6 years ago

@19majkel94 I agree with @j-keep it as separate package. I have quite a few projects on objection.js which I'd love to be able to use type-graphql with.

Generating schema form @Column sounds great-just keep it in @type-graphql/typeorm helper.

goldcaddy77 commented 6 years ago

@19majkel94 I'd love this as I'm using vesper right now thanks to it's opinionated ways of working with TypeORM. I like Vesper, but would really love to be able to generate the graphql schema with type-graphqls decorators and hook into your validation framework.

anodynos commented 5 years ago

My 5c is to keep it as it is, at least for now. And focus on other, much more useful features (eg #180 & #183 that is perhaps prerequisite).

I am saying it cause I've been using both TypeOrm & type-graphql since 0.14 (all inside latest NestJS & with many others goodies) and they play great together.

You can have a great separation of concerns:

The whole scheme works awesomely, I couldn't ask for more on this matter.

PS: Thanks for this AMAZING project. PS2: The same applies for the NestJS integration (or non-integration): it works perfectly as it is.

MichalLytek commented 5 years ago

keep it as it is, at least for now. And focus on other, much more useful features

That's why we have milestones to prioritize features: https://github.com/19majkel94/type-graphql/milestones?direction=asc&sort=due_date

it works perfect both the TypeORM lazy fetching & the GraphQL "awaited" fetch.

Now count the number of queries made for single request - it's not perfect at all. It's just a workaround. The goal is to provide auto generated optimized queries using #51.

It should work at least as good as Vesper integration.

The same applies for the NestJS integration (or non-integration): it works perfectly as it is.

AFAIK (#135), injecting services doesn't work, so it even doesn't work. Not speaking about "perfect" as there can be much better integration of decorators and nest features.

anodynos commented 5 years ago

Not speaking about "perfect" as

IMHO and experience, I am building a quite complex pet-project on top of NestJS, typeorm & type-graphql (with the advanced inheritance examples you have in the repo, amazing stuff, thank you!) and they all play really nice together in the end. To me, this is perfect!

Yes, one needs a few hackeries like this for services, but no major integration pains so far.

The goal is to provide auto generated optimized queries using #51.

For that, I can't disagree. But they say, first make it work well, then optimise :-)

It should work at least as good as Vesper integration.

Vesper is cute, has nice ideas, but its nowhere near as good as type-graphql packed with the 'defacto' general framework like NestJS.

My 5c goes to improving type-graphql core first, and then the integrations. But you're the boss! Thanks for your time and your awesome project! (BTW, how do we donate?)

MichalLytek commented 5 years ago

Thanks, it's nice to hear that ☺️

improving type-graphql core first, and then the integrations

That's the plan - see the milestones and their issues :)

how do we donate?

See #151. I was busy with writing my master's thesis and I had no time for this. But I will take care of that after 1.0 release. Big announcement and media noise will help gaining some attentions and supporters.

avkonst commented 5 years ago

My 5c is to keep it as it is, at least for now. And focus on other, much more useful features (eg #180 & #183 that is perhaps prerequisite).

I am saying it cause I've been using both TypeOrm & type-graphql since 0.14 (all inside latest NestJS & with many others goodies) and they play great together.

You can have a great separation of concerns:

  • @Entity + @ObjectType for both DB model and GraphQL entity. You can even define your relations as Promise<Foo> and it works perfect both the TypeORM lazy fetching & the GraphQL "awaited" fetch.
  • @InputType as a plain {} for your DTO / record / GraphQL input etc. You can add validators and all other goodies, works like charm.

The whole scheme works awesomely, I couldn't ask for more on this matter.

PS: Thanks for this AMAZING project. PS2: The same applies for the NestJS integration (or non-integration): it works perfectly as it is.

I do feel the same with apollo-server. And it is working great. Thanks for the project.

esistgut commented 5 years ago

Are there any news or best practices regarding this issue? I am exploring nest/type-graphql/typeorm for a new project but lazy relations are generating a massive amount of database queries. On the other hand eager relations seem to cause circular looping problems.

goldcaddy77 commented 5 years ago

@esistgut - check out how I do dataloaders in my project Warthog I have a complex example with tons of nesting and dataloaders here: https://github.com/goldcaddy77/warthog/tree/master/examples/7-feature-flags/src

Note: it does not use Nest.js currently, but uses TypeGraphQL, TypeORM, typedi and class-validator.

esistgut commented 5 years ago

@goldcaddy77 this seems to be based on a plain dataloader. Correct me if I'm wrong: this lowers the number of queries by "caching" them so the same query is not executed twice. Doesn't this force you to run the join of relations at the application level instead of the database level? And doesn't this still require an higher amount of queries than what could be done with proper database join resolution? The library suggested by @19majkel94 (https://github.com/Webtomizer/typeorm-loader) seems to implement the dataloader pattern but, as far as I understand, try to reduce multiple queries into a single one. Why didn't you use this?

goldcaddy77 commented 5 years ago

@esistgut - that's correct... This uses a standard dataloader. However dataloader doesn't get it's main performance benefit from caching, it gets it by batching all of the association records into a single query. So if you're trying to load all Posts and the User that created it, GraphQL would first load the Post records in one query, then the dataloader would aggregate all of the userId values and call something like `User.find({ where: { id_in: userIds }}). So you'd have 2 queries to load this.

I imagine the DB joining is more performant, but you give up a ton of flexibility. For instance, using dataloaders you can easily resolve data in a 3rd party system, a different microservice using a completely separate database, etc...

MichalLytek commented 5 years ago

And doesn't this still require an higher amount of queries than what could be done with proper database join resolution?

Still much less db queries that 40 REST API calls πŸ˜„