-
Notifications
You must be signed in to change notification settings - Fork 660
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 ApolloCompilerPlugin.schemaDocumentListener() #6165
Conversation
✅ Deploy Preview for apollo-android-docs canceled.
|
57cfaee
to
eec3e57
Compare
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.
Looks very good! I'll try it on the cache repo.
* @param outputDirectory the compiler output directory. This directory is shared with the compiler, make sure to use a specific | ||
* package name to avoid clobbering other files. | ||
*/ | ||
fun onSchemaDocument(schema: GQLDocument, outputDirectory: File) |
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 didn't catch it before, but would it make sense to also pass the packageName
configured in the service 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.
I left it out on purpose. The package name can be turing complete (because of packageNameFromFilePath
, etc...). How big of an issue is it to not have it? If really needed, one can always pass it as a ACP (Apollo Compiler Plugin) argument.
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.
Ah I see. Not a big issue, but I think this means the package name will have to be repeated in most cases.
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.
Another question - why not passing the Schema
rather than the GQLDocument
? To correctly look at directives I should use Schema.originalDirectiveName()
right?
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.
this means the package name will have to be repeated in most cases.
What about:
apollo {
service("service") {
packageName.set("com.example")
plugin(project(":cache-plugin")) {
argument("packageName", packageName.get())
}
}
}
And if evolution is a concern, hide it behind a java function:
apollo {
service("service") {
packageName.set("com.example")
configureCachePlugin(this, project(":cache-plugin"))
}
}
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.
why not passing the Schema rather than the GQLDocument?
Not sure. GQLDocument is overall more stable than Schema
but yea in your case, looks like you need schema
. I'll amend.
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.
#6166 for passing a Schema
instead of a GQLDocument
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.
packageName.get()
is totally acceptable, a dedicated fun as well 👍.
See here for an example