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

Cache pagination: add FieldNameGenerator and EmbeddedFieldsProvider #5772

Merged
merged 3 commits into from
Apr 8, 2024

Conversation

BoD
Copy link
Contributor

@BoD BoD commented Mar 29, 2024

In this PR, 2 APIs are added:

  • FieldNameGenerator to control how field names in records are computed. The default implementation uses CompiledField.nameWithArguments, and a Relay aware ConnectionFieldNameGenerator version is also provided. This unlocks cases where the pagination arguments are inside an input type. This is a reprise of this previous PR.
  • EmbeddedFieldProvider to control which fields should be embedded in a record. The default implementation uses CompiledNamedType.embeddedFields, and a Relay aware ConnectionEmbeddedFieldsProvider version is also provided.

Also, a CompilerArgumentDefinition is introduced, to more cleanly separate an argument value vs its definition (and sorry this makes this PR big with the .expected - I can separate this to another PR if preferred!). (this will go in a separate PR)

With this, the Pagination mechanism is usable without any directives in extra.graphqls (#5666), an example is given in ConnectionProgrammaticPaginationTest.

@BoD BoD requested a review from martinbonnin as a code owner March 29, 2024 17:25
Copy link

netlify bot commented Mar 29, 2024

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit 79ad292
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/660d907bedce2f0008d95eba

Comment on lines 269 to 272
public synthetic fun <init> (Ljava/lang/String;Lcom/apollographql/apollo3/api/Optional;ZZLkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getName ()Ljava/lang/String;
public synthetic fun <init> (Lcom/apollographql/apollo3/api/CompiledArgumentDefinition;Lcom/apollographql/apollo3/api/Optional;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getDefinition ()Lcom/apollographql/apollo3/api/CompiledArgumentDefinition;
public final fun getValue ()Lcom/apollographql/apollo3/api/Optional;
public final fun isKey ()Z
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep getName and isKeyas deprecated?

Comment on lines 36 to 38
CompiledArgument.Builder(CompiledArgumentDefinition.Builder("episode").build()).value("JEDI").build(),
CompiledArgument.Builder(CompiledArgumentDefinition.Builder("starsFloat").build()).value(9.9).build(),
CompiledArgument.Builder(CompiledArgumentDefinition.Builder("starsInt").build()).value(10).build()
Copy link
Contributor

Choose a reason for hiding this comment

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

What about generating the definitions only once and referencing them here?

First:

// type
class Query {
  // field
  class Reviews {
    companion object {
      // argument
      val episode = CompiledArgumentDefinition.Builder("episode")build()
      val starsFloat = CompiledArgumentDefinition.Builder("starsFloat").build()
    }
  }
  companion object {
    val type: ObjectType = ObjectType.Builder(name = "Query").build()
  }
}

Then

 CompiledArgument.Builder(Query.Reviews.episode).value("JEDI").build()

Comment on lines +9 to +10
interface EmbeddedFieldsProvider {
fun getEmbeddedFields(context: EmbeddedFieldsContext): List<String>
Copy link
Contributor

Choose a reason for hiding this comment

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

We could make this even more typesafe and also save a few string comparisons

interface CompiledFieldDefinition

// type
class Query {
  // field
  class Reviews {
    companion object {
      // argument
      val episode = CompiledArgumentDefinition.Builder("episode").build()
      val starsFloat = CompiledArgumentDefinition.Builder("starsFloat").build()
      
      val __definition = object: CompiledFieldDefinition {} 
    }
  }
  companion object {
    val type: ObjectType = ObjectType.Builder(name = "Query").build()
  }
}

And then

interface EmbeddedFieldsProvider {
  fun getEmbeddedFields(context: EmbeddedFieldsContext): List<CompiledFieldDefinition>

Not sure if it's worth it though...

@BoD BoD force-pushed the field-name-generator-and-embedded-fields-provider branch from 3543587 to a45c0ff Compare April 3, 2024 17:04
@BoD BoD merged commit fb8fe94 into main Apr 8, 2024
9 checks passed
@BoD BoD deleted the field-name-generator-and-embedded-fields-provider branch April 8, 2024 17:37
@BoD BoD mentioned this pull request Apr 22, 2024
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