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

feat: Types transformation utils #1295

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

xcfox
Copy link
Contributor

@xcfox xcfox commented Jun 3, 2022

This PR implements #453
Types transformation utils is a feature that has been on the back burner for a long time.

This PR comes with five utils types as:

  • PartialType: set all fields to nullable
  • RequiredType: set all fields to NON_NULL
  • PickType: only define specified field to a new type
  • OmitType: omit specified field to a new type
  • IntersectionType: combines two types into one new type

Use it like so:

@InputType()
class PartialObject {
  @Field()
  nullableStringField: string;
}

@ArgsType()
class RequiredObject {
  @Field()
  nonNullStringField: string;
}

@InterfaceType()
class PickedObject {
  @Field()
  pickedStringField: string;
}

@ObjectType()
class OmittedObject {
  @Field()
  OmittedStringField: string;
}

@ObjectType()
class SampleObject extends IntersectionType(
  IntersectionType(PartialType(PartialObject), RequiredType(RequiredObject)),
  IntersectionType(
    PickType(PickedObject, "pickedStringField"),
    OmitType(OmittedObject, "OmittedStringField"),
  ),
) {}

It work with class-validator and class-transformer
Actually, this code has been in our own repository for a year, it refers to the implementation of nestjs and #453 (comment)

In addition, I didn't find a way to implement mapping util that ensures type safety.

@MichalLytek
Copy link
Owner

Thank you for your PR ❤️

It work with class-validator and class-transformer

I think class-transformer is not used by TypeGraphQL. Only NestJS is using that.

What I don't like about such monkey-patching is that it does not work with other decorator-based libraries like TypeORM or joiful. Having such official helpers with such issues would be really bad.

What do you think about you publishing this helpers as a npm package and then linking it in the docs here?
So that all users know about this but at any time can switch to an alternative implementation supporting other decorator-based libs or even own homecrafted or forked solution? 😉

@xcfox
Copy link
Contributor Author

xcfox commented Jun 6, 2022

@MichalLytek Publishing this helpers as a npm package is a good idea. Do you think type-graphql-transformation is a good name?

@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Community 👨‍👧 Something initiated by a community labels Jun 7, 2022
@itpropro
Copy link

I like the idea of having a separate npm module for transformation. Can you also update or close #453 @MichalLytek so that anyone who watches there for implementations of your initial proposal knows what the status is? Thank you!

@ChrisLahaye
Copy link

Hi! We published our transformation utils to the type-graphql-utils package. It exports Pick, Required, Partial, and Omit. It doesn't have any specific code for class-validator like this PR, so I am not sure whether that works as we don't use it.

@okaforcj
Copy link

okaforcj commented Nov 6, 2022

hello @xcfox, @itpropro @MichalLytek since this has not been published or merged in, i copied the helper functions source-code for transformation into my application and everything works like a charm, my only concern is that i upgraded to typegraphql 2.0 as well. Everything still works fine, i just wanted to know if there were any concerns to adopting this or any issue i'd face in the near future because i see that it uses metadata to understand most of whats going on, so i dont know there might be breaking changes or general concerns

@okaforcj
Copy link

@xcfox @ChrisLahaye i think i found a bug, if you use PartialType/Partial on arrays, it seems like it strips the array and give its parameriterised value i.e. when using partial type on array of strings, it will just return a string as metadata for graphql, so my graphql types are not reflecting the actual types anymore even though they'd have been annotated before passing it through the PartialType function, Any thoughts on this please?

@ChrisLahaye
Copy link

@okaforcj Are you experiencing this issue on my library or the code here?

@okaforcj
Copy link

@okaforcj Are you experiencing this issue on my library or the code here?

both @ChrisLahaye, i tested your library as well before commenting, can you check it out or should i create a sample. like ive checked again and its happening for other util functions like required not just partial. Basically when you try to apply the util function with arrays, it drops the arrays in typegraphql/the graphql schema, even though the class in typescript world is still showing the array. I'm thinking it has something to do with how the metadata is been passed ?

@ChrisLahaye
Copy link

@okaforcj I see the same issue has been reported at ChrisLahaye/type-graphql-utils#3. I will look into it.

@ChrisLahaye
Copy link

@okaforcj I just released version 2.0.0 which should fix this issue. Credits to @caldarellial for reporting this issue and providing the fix.

@okaforcj
Copy link

@okaforcj I just released version 2.0.0 which should fix this issue. Credits to @caldarellial for reporting this issue and providing the fix.

that was quick! thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants