Mando75 / typeorm-graphql-loader

A query builder to easily resolve nested fields and relations for TypeORM-based GraphQL servers
https://gql-loader.bmuller.net
MIT License
57 stars 12 forks source link

Support nested/inherited types fields #8

Closed alexolivier closed 4 years ago

alexolivier commented 4 years ago

Currently, this doesn't seem to support nested or inherited fields as defined in the TypeORM Entity

@ObjectType()
@Entity()
export default class Thing1 {
  @PrimaryGeneratedColumn()
  id: number;

  @Field(type => NestedType)
  @Column(type => NestedType)
  nested: NestedType;
}

@ObjectType()
export default class NestedType {
  @Field(type => Date, { nullable: true })
  @Column({ nullable: true })
  someDate: Date;
}

The query it produces doesn't know that nestedSomeDate is a column in the DB

Mando75 commented 4 years ago

Thanks for opening the issue. To be totally honest, I didn't even know TypeORM had embedded types until now, so thanks for bringing this up! I might end up using them in some of my projects :)

As I mentioned in a previous comment, I am currently reworking the loader API to make it a bit more intuitive since people seem to be finding this package. I'll will absolutely look into supporting embedded types in that rewrite. I will say that I currently rely heavily on the metadata that TypeORM provides about it's entities in order to resolve the fields, so whether or not I am able to get it to work reliably with embedded types will depend on how much meta they provide about the embedded type. That being said, I'm confident that they would provide some means of identifying the embedded type so their inner query builders know how to load the data from the db.

I will leave this issue open until I am able to add it to the reworked API or I determine it is not possible.

Mando75 commented 4 years ago

Just an update. I was reading through the column metadata definitions and I believe that I have found a way to detect and account for nested types. I have already incorporated that solution into my refactor work. I will still need to do some testing around it, but it should be available in the next release.

alexolivier commented 4 years ago

Amazing! Any ETA @Mando75 ?

Mando75 commented 4 years ago

Hey Alex, I'm actually running into some issue getting this implemented and would love some insight.

I've figured out how to find embedded fields in the query info resolver, but I'm not sure how I can select them using the typeorm query builder. All the examples I've found use the FindOptions api which is not used in the data loader. Some examples of what doesn't work:

@Entity()
class Post {
   @PrimaryGeneratedColumn()  
   id: number

   @Column(type => PublicationData)
   publicationData: PublicationData
}

class PublicationData {
   @Column()
   datePublished: string

   @Column()
   views: number
}

I've tried querying in the following ways with no luck

    const post = await connection
      .getRepository(Post)
      .createQueryBuilder("post")
      .select("post.id")
      .addSelect("post.datePublished")
      .getOne();

Produces this query which looks correct:

SELECT "post"."id" AS "post_id", "post"."publicationDataDatepublished" FROM "post" "post" ORDER BY "post"."id" ASC

but the returned post doesn't have the data on it image

Just querying for addSelect("post.publicationData") fails as that column doesn't exist, and manually specifying the full column name doesn't return anything either.

It is very confusing because when I run the loader, it looks like it generates the correct query and is selecting the columns just like the simple example above but the data is not present on the returned entity. I'm not sure if I am doing something wrong or if this is a bug within typeorm. If you could provide any insight to how you would add selections for the embedded columns via the query builder it would be greatly appreciated.

alexolivier commented 4 years ago

Hey,

Nice work shipping the new version! Very clean API.

Been digging into this a bit - addSelect needs the full 'path' as defined by the entities:

    const post = await connection
      .getRepository(Post)
      .createQueryBuilder("post")
      .select("post.id")
      .addSelect("post.publicationData.datePublished")
      .getOne();

Internally it does the mapping to "post"."publicationDataDatepublished" so you should just be able to iterate down the tree and construct the 'path' from the SelectionSets

Mando75 commented 4 years ago

Perfect, thanks for your help digging into this. I have some time tomorrow where I should be able to try and get this working.

Mando75 commented 4 years ago

@alexolivier I was able to get the embedded fields working using the query format you provided. Thanks again for digging into that.

I did have a question on how you think it should be best implemented when mapping from a GraphQL type to a TypeORM entity. Right now I am assuming that just like in TypeORM, the embedded entity would have its own type, e.g. this entity structure in TypeORM

// embedded entity
class Address {
  @Column()
  street!: string;

  @Column()
  city!: string;

  @Column()
  state!: string;

  @Column()
  zip!: string;
}

// User entity that has an embedded address
@Entity()
class User extends BaseEntity {
  @Column()
  id: number;

  @Column(type => Address)
  address: Address
}

Would map to the following GraphQL Type definitions,

type Address {
  street: String!
  city: String!
  state: String!
  zip: String!
}

type User {
  id: ID!
  address: Address!
}

This is how I have it currently implemented on my feature branch. Does this fit your use case? As I mentioned, I haven't really used embedded entities myself, so I would love to get feedback from someone who does so I make sure this actually solves your need.

EDIT: Here is a Merge Request with the changes and some sample tests that verify that the data is loaded https://gitlab.com/Mando75/typeorm-graphql-loader/-/merge_requests/8

Mando75 commented 4 years ago

I went ahead and merged the above solution in v1.1.0.

valdisd commented 2 years ago

@Mando75

So, I searched for a while for how to implement this, and eventually it led me here and https://stackoverflow.com/questions/63029642/typeorm-composition-with-type-graphql-need-mixins/63261230#63261230 .

See, what I want and the Jerome from StackOverflow, and as I understand would make a lot more sense. Would be not to nest the attributes, but to inherit them. Just like the TypeORM does.

So this

type User { id: ID! address: Address! }

would become this:

type User { id: ID! street: String! city: String! state: String! zip: String! }

I think that based on the TypeORM embedded entity implementation that would make more sense. Is this possible right now?

Mando75 commented 2 years ago

@Mando75

So, I searched for a while for how to implement this, and eventually it led me here and https://stackoverflow.com/questions/63029642/typeorm-composition-with-type-graphql-need-mixins/63261230#63261230 .

See, what I want and the Jerome from StackOverflow, and as I understand would make a lot more sense. Would be not to nest the attributes, but to inherit them. Just like the TypeORM does.

So this

type User { id: ID! address: Address! }

would become this:

type User { id: ID! street: String! city: String! state: String! zip: String! }

I think that based on the TypeORM embedded entity implementation that would make more sense. Is this possible right now?

@valdisd TypeORM does not inherit the attributes of embedded entities. It provides them under a nested type. Given the following TypeORM entities

// Embedded address entity
class Address {
  @Column()
  street: string

  @Column()
  city: string

  @Column()
  postCode: string
}

// User Entity
@Entity()
class User extends BaseEntity {
  @PrimaryGeneratedColumn()
   id: number

   @Column(() => Address)
   address: Address
}

If you want to access the user's post code, you have to go via address, you cannot access it directly from the User entity:

const user = await connection.getRepository(User).findOne()
// works
let postCode = user.address.postCode

// will be undefined/throw error if using TypeScript as property `postCode` does not exist on type User
postCode = user.postCode

This is why the loader requires the embedded entity to have a corresponding nested field in the GraphQL type. While it is true that embedded entities will live in the same table as their parent entity, you cannot access their properties directly from the parent. You must go through the nested field. The loader expects your GraphQL schema to match the same structure as your TypeORM entity, hence requiring a nested GraphQL type to match the nested Embedded Entity.