-
Notifications
You must be signed in to change notification settings - Fork 654
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
Add basic support for fragments #27
Conversation
import com.apollostack.compiler.ir.Field | ||
import com.cesarferreira.pluralize.singularize | ||
import com.apollostack.compiler.ir.Fragment | ||
import com.squareup.javapoet.TypeSpec | ||
import javax.lang.model.element.Modifier | ||
|
||
class OperationTypeSpecBuilder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we merge this with Operation
class ? To be consistent turn Operation
class to follow the same pattern as TypeDeclaration
and Fragment
, extends CodeGenerator
and remove this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I thought about doing that. Sounds like it could be a good idea for consistency
import com.squareup.javapoet.TypeSpec | ||
import javax.lang.model.element.Modifier | ||
|
||
class OperationTypeSpecBuilder( | ||
val operationName: String, | ||
val fields: List<Field>) { | ||
val fields: List<Field>, | ||
val allFragments: List<Fragment>) : CodeGenerator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about to wrap this to next line for more readability, so it will look like this:
data class Fragment(
val fragmentName: String,
val source: String,
val typeCondition: String,
val fields: List<Field>,
val fragmentsSpread: List<String>,
val inlineFragments: List<String>,
val fragmentsReferenced: List<String>
) : CodeGenerator {
/** Returns a Java method that returns the interface represented by this Fragment object. */
private fun toMethodSpec() =
MethodSpec.methodBuilder(fragmentName.decapitalize())
.returns(ClassName.get("", fragmentName.capitalize()))
.addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
.build()
....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure!
Awesome this is getting shape |
TypeSpec.interfaceBuilder(field.normalizedName()) | ||
.addModifiers(Modifier.PUBLIC, Modifier.STATIC) | ||
.addMethod(Fragment.genericAccessorMethodSpec()) | ||
.addType(Fragment.genericInterfaceSpec(fragments)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that we are regenerating the same fragments all over again if fields reference to the same fragments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, like I said, this is pretty basic and there's a bunch of things to improve. This is one of them 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok could you pls drop TODO here, that we won't forget it later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
.addMethods(fields.map(Field::toMethodSpec)) | ||
.build() | ||
|
||
companion object { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this go to OperationTypeSpecBuilder
so that the name of interface and method will be decided by reference class (OperationTypeSpecBuilder
) and not hardcoded here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, which method are you referring to exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry I meant about this genericAccessorMethodSpec
. The intention of this is to not make decision about the name of method that returns fragments here but move to the guy who is actually uses it, in our case OperationTypeSpecBuilder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, will move it
I tried next query:
And appears it doesn't generate |
Fixes #26