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

Allow custom scalars targets in FakeResolver return types #4359

Merged
merged 3 commits into from
Aug 29, 2022

Conversation

martinbonnin
Copy link
Contributor

Previously, compile-time adapters required resolveLeaf to return a Json-like value. This break the symmetry with runtime adapters and is more cumbersome to use.

To allow this, we now generate a customScalarAdapters property that contains all compile-time adapters that we can use to automatically serialize when needed.

Random thoughts: the compile-time vs run-time difference make all this code pretty cumbersome. In the future, we should think of ways to make everything a compile-time thing. Maybe with KSP:

@ApolloAdapter // annotation that will signal the adapter to use
object MyLongAdapter: Adapter<MyLong> {
  override fun fromJson(reader: JsonReader, customScalarAdapters: CustomScalarAdapters): MyLong {
    return MyLong(reader.nextString()!!.toLong())
  }

  override fun toJson(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, value: MyLong) {
    writer.value(value.value.toString())
  }
}

@netlify
Copy link

netlify bot commented Aug 26, 2022

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit e6665d3
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/6308c88b1dc5300008802597

}
} else {
leafValue
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the meat of the change

* A GraphQL scalar type that is mapped to a Kotlin. This is named "Custom" for historical reasons
* but is also used for builtin scalars
*
* TODO v4: rename this to ScalarType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started adding "TODO v4" for stuff that will need to be reworked. I don't think we had any marker previously?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea!
(I don't think we did)

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.

Very cool! 👍

@martinbonnin martinbonnin changed the title Allo custom scalars targets in FakeResolver return types Allow custom scalars targets in FakeResolver return types Aug 29, 2022
@martinbonnin martinbonnin merged commit efc90e4 into main Aug 29, 2022
@martinbonnin martinbonnin deleted the use-target-for-custom-scalars branch August 29, 2022 23:05
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