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

Generic Type #245

Closed
wants to merge 14 commits into from
Closed

Generic Type #245

wants to merge 14 commits into from

Conversation

owengombas
Copy link

Use Case:

I've made an example into examples/generic-type, with this you can see what is the goal of the feature.
Let suppose that you have multiple ObjectType, a QueryResponse class that is generic and has a dynamic type field :

My ObjectTypes:
image
image

My QueryResponse generic class that has a generic property items :
image

So now if you want to return a QueryResponse in a cars query (QueryResponse) and the same for a persons query (QueryResponse). So you do not need to redeclare a new class to type staticly items field.

You can use the new type-graphql generic typing feature in queries like this (Also usable as a ArgsType or InputType: see examples/generic-type):
image

Compiled queries
image
image

@codecov
Copy link

codecov bot commented Jan 31, 2019

Codecov Report

Merging #245 into master will decrease coverage by 6.36%.
The diff coverage is 45.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
- Coverage   95.88%   89.51%   -6.37%     
==========================================
  Files          67       69       +2     
  Lines        1045     1164     +119     
  Branches      183      214      +31     
==========================================
+ Hits         1002     1042      +40     
- Misses         42      121      +79     
  Partials        1        1
Impacted Files Coverage Δ
src/decorators/Args.ts 100% <100%> (ø) ⬆️
src/decorators/Field.ts 100% <100%> (ø) ⬆️
src/decorators/index.ts 100% <100%> (ø) ⬆️
src/metadata/metadata-storage.ts 80.12% <32.65%> (-19.88%) ⬇️
src/decorators/GenericType.ts 33.33% <33.33%> (ø)
src/decorators/GenericField.ts 37.5% <37.5%> (ø)
src/schema/schema-generator.ts 83.33% <50%> (-13.8%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e5ea9d...dd5d170. Read the comment docs.

@MichalLytek
Copy link
Owner

@OwenCalvin
I really appreciate your work, looks like you've spend a lot of time 💪

But I wasn't asking for help with generics - as you can see in #180, there's no "Help wanted" badge and I'm assigned to this issue to do this by my own.

The proposed API is definitely not the one I would like to introduce. It's too complicated, missleading (type => and now { type: } too), there's too much magic going behind and not enough typesafe.

I have different design goals and feature implementations/bug solution in my head, so I can't accept your PR.

I've placed and info in the readme long, long time ago to prevent this kind of situation:

If you want to add a new big feature, please create a proposal first, where we can discuss the idea and implementation details. This will prevent wasting of your time if the PR be rejected.

But feel free to continue your work in your fork @owencalvin/type-graphql or maybe as a different framework (like @goldcaddy77 warthog if you need this kind on feature and you like this API.

@MichalLytek MichalLytek closed this Feb 1, 2019
@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Community 👨‍👧 Something initiated by a community labels Feb 1, 2019
@owengombas
Copy link
Author

Okay no problem, I needed this so I made it :)
I will continue my work but I keep an eye on you implementation :)

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.

2 participants