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

POC: nested input support #452

Closed
wants to merge 1 commit into from
Closed

POC: nested input support #452

wants to merge 1 commit into from

Conversation

j
Copy link
Contributor

@j j commented Oct 29, 2019

This is an attempt at fixing inputs.

I'm doing this by iterating over the input type fields, checking if the field's type is an input type, and if so, hydrating the argument into the class. I attempted to make it more performant by caching the tree based off the param metadata's type.

@codecov
Copy link

codecov bot commented Oct 29, 2019

Codecov Report

Merging #452 into master will increase coverage by 0.09%.
The diff coverage is 97.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
+ Coverage   94.65%   94.75%   +0.09%     
==========================================
  Files          71       72       +1     
  Lines        1160     1201      +41     
  Branches      218      227       +9     
==========================================
+ Hits         1098     1138      +40     
  Misses         59       59              
- Partials        3        4       +1
Impacted Files Coverage Δ
src/resolvers/helpers.ts 100% <100%> (ø) ⬆️
src/resolvers/convert-arg.ts 97.5% <97.5%> (ø)

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 98e9839...ac6152a. Read the comment docs.

@j j changed the title feat: nested input support POC: nested input support Oct 29, 2019
@j
Copy link
Contributor Author

j commented Oct 29, 2019

/cc @MichalLytek

@MichalLytek MichalLytek self-requested a review November 2, 2019 15:38
@MichalLytek MichalLytek added Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request labels Nov 2, 2019
@MichalLytek MichalLytek added this to the 1.0.0 release milestone Nov 2, 2019
Copy link
Owner

@MichalLytek MichalLytek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the solution and it fails on transforming arrays - just breaks completely and the data is lost.

@MichalLytek
Copy link
Owner

MichalLytek commented Nov 10, 2019

@j Your idea is good! 💪 I will try to work on this to support all cases like @Args, arrays, scalars and nulls.

@MichalLytek
Copy link
Owner

Closing in favor of #462 🔒

@MichalLytek MichalLytek added the Wontfix ❌ This will not be worked on label Nov 10, 2019
@MichalLytek MichalLytek removed this from the 1.0.0 release milestone Nov 10, 2019
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 Wontfix ❌ This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants