-
-
Notifications
You must be signed in to change notification settings - Fork 677
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
base: master
Are you sure you want to change the base?
Conversation
Thank you for your PR ❤️
I think What I don't like about such monkey-patching is that it does not work with other decorator-based libraries like TypeORM or What do you think about you publishing this helpers as a npm package and then linking it in the docs here? |
@MichalLytek Publishing this helpers as a npm package is a good idea. Do you think |
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! |
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. |
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 |
@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? |
@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 ? |
@okaforcj I see the same issue has been reported at ChrisLahaye/type-graphql-utils#3. I will look into it. |
@okaforcj I just released version |
that was quick! thanks for the help! |
a190360
to
f50d1d0
Compare
6721362
to
e5836d9
Compare
6e1c81e
to
c8d2e2d
Compare
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:
Use it like so:
It work with
class-validator
andclass-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.