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

Make the compiler accept Json configuration files #5516

Merged
merged 11 commits into from Jan 12, 2024
Merged

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Jan 9, 2024

The compiler can be used from different contexts:

  • from the Gradle plugin (current)
  • from a Gradle worker (needed for compiler APIs)
  • from Maven

In order to make it easier to work in these different contexts, make the compiler accept JSON configuration files.

Instead of having conventions and default behaviours in the Gradle Plugin, they are now in the Compiler.

Copy link

netlify bot commented Jan 9, 2024

Deploy Preview for apollo-android-docs ready!

Name Link
🔨 Latest commit b4fdbeb
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/65a038ea54001d0008d6d132
😎 Deploy Preview https://deploy-preview-5516--apollo-android-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -144,7 +144,6 @@ kotlin-node = "org.jetbrains.kotlin-wrappers:kotlin-node:18.16.12-pre.634"
kotlinx-serialization-plugin = { group = "org.jetbrains.kotlin", name = "kotlin-serialization", version.ref = "kotlin-plugin" }
kotlinx-serialization-core = { group = "org.jetbrains.kotlinx", name = "kotlinx-serialization-core", version.ref = "kotlinx-serialization-runtime" }
kotlinx-serialization-json = { group = "org.jetbrains.kotlinx", name = "kotlinx-serialization-json", version.ref = "kotlinx-serialization-runtime" }
kotlinx-serialization-json-okio = { group = "org.jetbrains.kotlinx", name = "kotlinx-serialization-json-okio", version.ref = "kotlinx-serialization-runtime" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not stable so removing from the apollo-compiler API since users may start using the apollo-compiler API directly with this PR (some might have done so already but apollo-compiler was mostly used by the Gradle plugin so far)

Comment on lines -42 to -43
public static final fun toCodegenMetadata (Ljava/io/File;)Lcom/apollographql/apollo3/compiler/CodegenMetadata;
public static final fun writeTo (Lcom/apollographql/apollo3/compiler/CodegenMetadata;Ljava/io/File;)V
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Breaking changes everywhere in this file. I declare them OK cause the apollo-compiler API was not really meant to be accessed outside the Gradle Plugin.

Comment on lines -260 to -269
val duplicates = irOperations.flatMapIndexed { index, irOperations1 ->
irOperations1.fragmentDefinitions.map { it.name to irOperationsFiles.get(index).path }
}.groupBy { it.first }.mapValues { it.value.map { it.second } }
.entries
.filter { it.value.size > 1 }
.flatMap { entry ->
entry.value.map {
"- ${entry.key}: $it"
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out this was not really needed as the check is already done here.

This means it's ok to have clashing fragments in sibling modules as long as they have different package names.

Comment on lines +136 to +140
val packageNameGenerator = packageNameGenerator ?: packageNameGenerator(
codegenSchemaOptions.packageName,
codegenSchemaOptions.packageNamesFromFilePaths,
packageNameRoots
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the prep work for moving packageNameGenerator to a ServiceLoader. We would still support packageName and packageNamesFromFilePaths as declarative options present in the JSON files that can be overriden by a compiler plugin if needed.

@martinbonnin
Copy link
Contributor Author

This should now be ready for review. Initial description updated.

@martinbonnin martinbonnin marked this pull request as ready for review January 11, 2024 18:52
Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

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

⚙️

@martinbonnin martinbonnin merged commit d16c644 into main Jan 12, 2024
9 checks passed
@martinbonnin martinbonnin deleted the json-config branch January 12, 2024 13:26
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.

None yet

2 participants