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

Data Builders for fragments and customScalarAdapters for map builders #4338

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

martinbonnin
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Aug 12, 2022

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit 6d13944
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/62f621f71a8f3300097f3768

resolver: FakeResolver,
type: CompiledType
): T {
val map = if (block == null) {
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 has to be a relatively different code path because fragments can be on interfaces types so we can't know in advance the builder to instanciate:

FragmentImpl.Data {
  // buildDroid is also an option
  buildHuman {
    ...
  }
}
QueryImpl.Data {
  // here the receiver is always QueryBuilder
  name = ...
}

@@ -25,7 +26,7 @@ class IncludeTest {
fun includeVariableTrue() = runBlocking {
val operation = GetCatIncludeVariableQuery(withCat = true)

val data = buildQuery {
val data = GlobalBuilder.buildQuery {
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 slightly less ergonomic but it's the price to pay to allow runtime adapters if needed:

Builder(customScalarAdapters).buildQuery {
  ...
}

HeroWithFriendsFragment(
id = "2001",
name = "R222-D222",
HeroWithFriendsFragmentImpl.Data {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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 45650f9 into main Aug 12, 2022
@martinbonnin martinbonnin deleted the adapters-scope branch August 12, 2022 12:13
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.

2 participants