fix non nullable nested object marked as nullable #368
Conversation
Codecov Report
@@ Coverage Diff @@
## master #368 +/- ##
============================================
- Coverage 94.46% 94.45% -0.01%
+ Complexity 323 322 -1
============================================
Files 97 96 -1
Lines 1139 1137 -2
Branches 204 204
============================================
- Hits 1076 1074 -2
Misses 22 22
Partials 41 41
Continue to review full report at Codecov.
|
| @@ -47,12 +47,12 @@ internal open class TypeBuilder constructor(protected val generator: SchemaGener | |||
|
|
|||
| // Do not call the hook on GraphQLTypeReference as we have not generated the type yet | |||
| val unwrappedType = GraphQLTypeUtil.unwrapType(graphQLType).lastElement() | |||
| val typeWithNullability = graphQLType.wrapInNonNull(type) | |||
There was a problem hiding this comment.
This would throw an exception if I had a type returned from the hooks that correctly had it wrapped in a GraphQLNonNull right? I think we can refactor the wrapInNonNull method to have the logic to check if it is already non-null. If it is just return it instead of throwing an exception
@dariuszkuc What do you think?
There was a problem hiding this comment.
Not sure what the original purpose of throwing that error was, but it would have thrown an error if a GraphQLNonNull type contained anything besides a GraphQLTypeReference. I think the only ways for a GraphQLNonNull type to get into that method are to create a scalar type that is wrapped in GraphQLNonNull or implement onRewireGraphQLType to wrap a type in GraphQLNonNull. Idk if there is a valid use case for either one of those.
There was a problem hiding this comment.
I don't think there is. Let's just change the logic and remove the exception and all its test cases
smyrick
added
changes: patch
* fix non nullable nested object marked as nullable * update federated schema nested test expected result * fix lint issues * return same type when GraphqlNonNull passed to wrapInNonNull
📝 Description
Fix non nullable nested object marked as nullable and add/update tests.
🔗 Related Issues
This resolves #367