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

🍂 3.0.0 fall cleanup of the Gradle Plugin 🍂 #2668

Merged
merged 5 commits into from
Oct 20, 2020

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Oct 16, 2020

At a high level, the main change is that the plugin doesn't try to create a GraphQL source set per Kotlin source set anymore. Instead, there is now a 1:1 mapping between a service and a GraphQL source set. By default, files are are searched under src/main/graphql or src/commonMain/graphql but this is just a convention that can be overriden.

Trying to map GraphQL to Java/Kotlin source sets was confusing and created issues like #1981 or special cases for the test source sets where we would end up with duplicate classes in the classpath. Also the schema resolution rules were becoming super complex and hard to maintain.

Having one GraphQL service per sourceSet is a simple mapping that allows the files/schema resolution rules to be way easier than they used to be, removing the need for schemaPath vs schemaFile, CompilationUnit, ApolloVariant, ...

In more details, a few points worth noting:

Android variants handling

The custom Android variants handling code has been removed. This means that the plugin will not look into the different src/${flavor}/graphql directories by default. I'm hoping that in the very vast majority of case this will not an issue because there is rarely a need for different queries. This will even save some compilation time by not generating sources for each variant. If per-variant queries are still required, this can still be achieved either with:

  1. R8: define all the queries in the main variant and have R8 remove the ones that are unused in other variants.
  2. Or define different services and wire them to the variants manually (see below)

Custom task wiring

For the case where finer control over how the generated sources are used is required, the Service now offers a withOutputDir action that exposes a Task and a Provider<Directory> that you can forward to other parts of the build as needed. Coupled with Service.outputDir, this allows more complex scenarios like registering generated sources to specific Android variants or to a test source set:

apollo {
  service("starwars") {
    graphqlSourceDirectorySet.srcDir("src/test/graphql/starwars")
    withOutputDir {
      val kotlinProjectExtension = project.extensions.get("kotlin") as KotlinProjectExtension
      // add the starwars generated sources to the test source set
      kotlinProjectExtension.sourceSets.getByName("test").kotlin.srcDir(outputDir)
    }
  }
}

Default service

This PR also gets rid of the whole CompilerParams inheritance/overriding because:

  1. the behaviour wasn't always obvious
  2. explicit is better than implicit
  3. it didn't make any sense for some arguments like customTypeMapping that are specific to a schema to be shared by a common ancestor.

To keep things simple and keep some of the previous behaviour, for simple cases where there is only one schema, a default service is created at the root of the apollo {} block:

apollo {
  // this is still working and will generate generated/source/apollo/service/...
  sourceFolder.set("starwars") 
}

Mixing implicit and explicit services will fail to make this new behaviour explicit:

apollo {
  sourceFolder.set("starwars") 
  service("github") {
    // this will fail. You can either remove `sourceFolder.set("starwars")` above or create an explicit "starwars" service
    sourceFolder.set("github")
  }
}

(Almost) no more afterEvaluate {}

The plugin is one step closer to removing afterEvaluate completely. Tasks are now registered 100% reactively. There are still 2 afterEvaluate: one for the defaultService and the other to make sure the Android and Kotlin plugins have settled. This last one is maybe not even required but it felt safer to keep it.

closes #2563

@martinbonnin martinbonnin changed the base branch from main to dev-3.x October 16, 2020 18:27

import org.gradle.api.provider.Property

interface Registry {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this class used for just grab GraphQL schema from Apollo registry? Should we add some class docs?

Copy link
Contributor Author

@martinbonnin martinbonnin Oct 18, 2020

Choose a reason for hiding this comment

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

Yes, it's the equivalent of apollo client:download-schema in the npm cli. I'll add some class docs 👍 .

Copy link
Contributor

@sav007 sav007 left a comment

Choose a reason for hiding this comment

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

That great clean up. TBH I was lost with compilation units and inheritance in the past.

At a high level, the main change is that the plugin doesn't try to create a GraphQL source set per Kotlin source set anymore. Instead, there is now a 1:1 mapping between a service and a GraphQL source set. By default, files are are searched under src/main/graphql or src/commonMain/graphql but this is just a convention that can be overriden.

Trying to map GraphQL source sets was confusing and created issues like #1981 or special cases for the test source sets where we would end up with duplicate classes in the classpath. Also the schema resolution rules were becoming super complex and hard to maintain.

Having one GraphQL service per service is a simple mapping that allows the files/schema resolution rules to be way easier than they used to be, removing the need for schemaPath vs schemaFile, CompilationUnit, ApolloVariant, ...
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.

[Plugin] Robustify no-source handling
2 participants