Skip to content

Conversation

@zypus
Copy link

@zypus zypus commented Apr 3, 2021

The pull request proposes some changes I introduced to implement the functionality I mentioned in issue #139 .

The changes don't break any of the existing code, however I touched upon a couple of files and I am not super satisfied with the outcome. Ie. I need to catch some internal Kotlin reflection exceptions to prevent the checks for a valid return type from failing early in the resolver construction. Because I will supply the expected return type explicitly in a secondary call in the ResolverDSL, similar to the withArgs. The explicit type resolution works similar to the DataLoaderProperty. Also, I had to add another method to the Target interface in the ResolverDSL which has the unfortunate side effect that there are now some methods in some of the other subclasses, where those function currently do nothing, because I didn't see the need for the generic support there (might be useful there as well though).

The code works now as I envisioned as can be shown in the tests I added. However, I am not 100% sure if the code is really merge ready. Nonetheless I hope this pull request is still somewhat helpful.

@jeggy
Copy link
Member

jeggy commented Apr 5, 2021

This implementation looks fine for now. I would preferably not have the need to provide .returns<>(), but I have no other solution for that currently.

As JCenter has closed down I will focus on getting KGraphQL published to MavenCentral(#134) and this will be released when that has been implemented

@jeggy jeggy merged commit 3b6501d into aPureBase:main Apr 5, 2021
@zypus
Copy link
Author

zypus commented Apr 10, 2021

I agree my solution is definitely far from ideal. I just went that route so I don't have to break any existing interfaces.

Probably one possible solution would be to make the resolver functions inline with reified generics themselves. However, this is probably not backwards compatible and it would expose some internals, because of the nature of inline functions.

@zypus zypus deleted the genericQueryCreation branch April 10, 2021 11:20
@zypus zypus mentioned this pull request Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants