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

TypeORM integration plugin #44

Open
MichalLytek opened this issue Mar 16, 2018 · 14 comments
Open

TypeORM integration plugin #44

MichalLytek opened this issue Mar 16, 2018 · 14 comments
Labels
Enhancement 🆕 New feature or request
Milestone

Comments

@MichalLytek
Copy link
Owner

MichalLytek commented Mar 16, 2018

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 ❓

@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Discussion 💬 Brainstorm about the idea labels Mar 16, 2018
@MichalLytek MichalLytek added this to the Future release milestone Mar 19, 2018
@j
Copy link
Contributor

j commented Apr 2, 2018

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
Copy link
Owner Author

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
Copy link
Contributor

capaj commented May 29, 2018

@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
Copy link

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

@MichalLytek MichalLytek removed the Discussion 💬 Brainstorm about the idea label Oct 24, 2018
@MichalLytek MichalLytek changed the title [Discussion] Better TypeORM integration TypeORM integration plugin Oct 24, 2018
@anodynos
Copy link

anodynos commented Nov 8, 2018

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.

@MichalLytek
Copy link
Owner Author

MichalLytek commented Nov 8, 2018

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
Copy link

anodynos commented Nov 8, 2018

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
Copy link
Owner Author

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
Copy link

avkonst commented Nov 8, 2018

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
Copy link

esistgut commented Jul 3, 2019

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
Copy link

@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
Copy link

esistgut commented Jul 3, 2019

@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
Copy link

@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
Copy link
Owner Author

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 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement 🆕 New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants