-
Notifications
You must be signed in to change notification settings - Fork 646
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
Allow capitalized field names if flattenModels is true #4154
Conversation
✅ Deploy Preview for apollo-android-docs canceled.
|
apollo-ast/src/main/kotlin/com/apollographql/apollo3/ast/api.kt
Outdated
Show resolved
Hide resolved
apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/CodegenLayout.kt
Outdated
Show resolved
Hide resolved
...ler/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/java/adapter/AdapterCommon.kt
Outdated
Show resolved
Hide resolved
@@ -68,7 +68,8 @@ class CompiledSelectionsBuilder( | |||
} | |||
|
|||
private fun List<GQLSelection>.walk(name: String, private: Boolean, parentType: String): List<FieldSpec> { | |||
val propertyName = resolveNameClashes(usedNames, context.layout.propertyName(name)) | |||
val maybeEscapedName = if (private) context.layout.escapedPropertyName(name) else context.layout.propertyName(name) | |||
val propertyName = resolveNameClashes(usedNames, maybeEscapedName) |
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 like the only case where this is public is the root
field? In that case, you could rename the root field to __root
(technically source breaking but I think that's ok?) and escape indiscriminately, just like for variables?
All in all, I think I'd do this:
- __typename -> never escape
- __root -> never escape
- variables -> always escape
- add a new
compiledSelectionsName()
that would always escape.
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.
Digging into this again. Do you remember why we needed to escape the compiled selections? They look like this:
public object TestQuerySelections {
private val __onDroid: List<CompiledSelection> = listOf(..)
private val __hero: List<CompiledSelection> = listOf(..)
// ...
public val __root: List<CompiledSelection> = listOf(..)
}
This is all private (except __root) and this file has some code to resolve name clashes:
Line 51 in 89a439c
private fun resolveNameClashes(usedNames: MutableSet<String>, modelName: String): String { |
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.
You're right, it looks like escaping is unnecessary - not sure why we did this 😅 Maybe we overlooked the fact that only __root
is public?
apollo-ast/src/main/kotlin/com/apollographql/apollo3/ast/Issue.kt
Outdated
Show resolved
Hide resolved
apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/ApolloCompiler.kt
Outdated
Show resolved
Hide resolved
apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/checkNoCapitalizedFields.kt
Outdated
Show resolved
Hide resolved
apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/checkNoCapitalizedFields.kt
Outdated
Show resolved
Hide resolved
apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/CodegenLayout.kt
Show resolved
Hide resolved
...ler/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/java/adapter/AdapterCommon.kt
Outdated
Show resolved
Hide resolved
...tlin/com/apollographql/apollo3/compiler/codegen/java/selections/CompiledSelectionsBuilder.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Martin Bonnin <martin@mbonnin.net>
4dc8cad
to
96caf49
Compare
...tlin/com/apollographql/apollo3/compiler/codegen/java/selections/CompiledSelectionsBuilder.kt
Outdated
Show resolved
Hide resolved
...tlin/com/apollographql/apollo3/compiler/codegen/java/selections/CompiledSelectionsBuilder.kt
Outdated
Show resolved
Hide resolved
...in/com/apollographql/apollo3/compiler/codegen/kotlin/selections/CompiledSelectionsBuilder.kt
Outdated
Show resolved
Hide resolved
...ntlr_tokens/java/operationBased/antlr_tokens/adapter/TestQuery_ResponseAdapter.java.expected
Show resolved
Hide resolved
🚀 |
Related to #4152