Is there an example of a `ExecutableDocumentTransf...

# apollo-kotlin

[wasyl]

Is there an example of a ExecutableDocumentTransform that would add `id`s to selections on types that have id field in the schema, but it's not in the current selection set? I saw https://github.com/apollographql/apollo-kotlin/blob/415255a8d4203b2714924584a4e80f2283274769/tests/compiler-plugins/add-field/src/main/kotlin/hooks/TestPlugin.kt#L44 and it got me somewhere, but I'm struggling to get and pass around the type I'm currently processing (so that I can look up in schema whether it has an id field defined)

[wasyl]

I'm doing it rather blindly tbh because I do not understand what schema.typeDefinition and definitionFromScope really do. I have a vague idea what I want them to be doing, but when I try to get current GQLTypeDefinition it seems that I need to understand what types I can encounter and how to map them to the type def

[bod]

it.definitionFromScope(schema, parentType) gives you the schema definition of the field from a field in a selection. From that you can get the GQLType of the field

[bod]

And from that you can get all the field definitions of that type (and see if there's an id one)

[mbonnin]

I've been doing that so many times 😅

[mbonnin]

At some point we should get a proper traversal API

[mbonnin]

But ++ on it.definitionFromScope(schema, parentType), this is the way currently

[wasyl]

well I suppose but what I'm confused about is how to call the it.definitionFromScope(schema, parentType) properly

[wasyl]

this is what I start with and then what 😄

[wasyl]

hm schema.typeDefinition(type.rawType().name) seems to work 👀

[mbonnin]

There is something very similar here

[mbonnin]

The trick is with inline fragments, the parentType is not the type of the field so it requires a bit of care

{
  node {
    # parentType is Node
    ... on Product {
      # parentType is Product
      price
    }
  }
}

[mbonnin]

To this day, I get confused when trying to write this

[wasyl]

Yes I saw that too, but that was pretty overwhelming

[wasyl]

The inline fragments thingy is it.typeCondition?.name ?: parentType? I wrote

is GQLInlineFragment -> it.copy(
  selections = it.selections.addIdIfNeeded(
    schema = schema,
    parentType = it.typeCondition?.name?.let { schema.typeDefinition(it) } ?: parentType,
  ),
)

[mbonnin]

Yea, graphQL operations are a "tree of trees". You have a tree of fields. And for each field, you have a tree of type conditions, it takes some time to get used to.

[mbonnin]

The inline fragments thingy is it.typeCondition?.name ?: parentType?

From a cursory look, that looks ok

[mbonnin]

(Because to add to the confusion, anonymous fragments are a thing)

[mbonnin]

{
  node {
    # parentType is Node
    ... {
      # anonymous fragment here
      id
    }
  }
}

[wasyl]

uhh. Alright I got something working but I think I'm duplicating the IDs that are already in one fragment in the selection set

[wasyl]

(btw I wouldn't be mad if add IDs to types that have them was a built-in utility, shouldn't that be ultra common use case?)

[mbonnin]

That was the idea behind @typePolicy(keyFields: "id")

[mbonnin]

But agreed, we need more utilities around this

[mbonnin]

The whole apollo-ast thing needs higher level APIs

[wasyl]

That was the idea behind @typePolicy(keyFields: "id")

I thought this is to define which field is the ID for normalized cache? blob thinking fast What I'm trying to do is making sure we fetch id field if it's defined in the schema, even if people don't write it themselves in the queries

[mbonnin]

Yes but it's somewhat related I think?

[mbonnin]

ids are used for normalized cache usually?

[mbonnin]

Or do you need them for something else?

[bod]

maybe it's not clear that when using @typePolicy, these ids are automatically added to selections (by the piece of code you pointed to 🙂)

[mbonnin]

Ah yes, I should have mentioned that

[wasyl]

when using @typePolicy, these ids are automatically added to selections

exactly that wasn't clear to me 😄

[wasyl]

I'm torn, mostly because there seems to be something I should use, but I really want a no-effort solution here. @typePolicy needs to be written on every type right?

[mbonnin]

If you have a Node interface you can add it to the interface but if not, then yea, it becomes hard to maintain

[wasyl]

oh hey maybe I do have a Node 👀

[wasyl]

meh not always

[mbonnin]

Another option would be to add a schema transform to add @typePolicy(keyFields: "id") and then leave @typePolicy add the field automatically down the road, this is probably easier to write than the addRequiredIds()

[wasyl]

That'd be registerSchemaCodeGenerator?

[mbonnin]

More registerSchemaTransform

[mbonnin]

The SchemaCodeGenerator happens later down the road to give users an opportunity to write some KotlinPoet code based on the schema

[wasyl]

hm I'm in ApolloCompilerPlugin and don't see the registerSchemaTransform. Old version maybe?

[mbonnin]

Ah yes, that's proabbly v5 only

[mbonnin]

You can preprocess your schema in a custom Gradle task

[mbonnin]

That works too. I was actually pondering whether we need a plugin API for that because the schema is really an input to the compiler, everything can happen upstream

[mbonnin]

(but writing Gradle tasks isn't fun)

[wasyl]

I still need some APIs to transform the AST so I appreciate a plugin mechanism

[mbonnin]

You can pull apollo-ast in your buildscript classpath but yea, it starts to become a lot

[mbonnin]

Another advantage of using @typePolicy is that we could potentially add the ids post codegen.
I think this could be beneficial because the user would only see the properties that they explicitely requested. No more relying on the compiler to add fields for you and break when you change the addTypename value

[mbonnin]

This is obviously a double edge sword but centralizing the logic to add ids sounds desirable

[wasyl]

you mean the user will see the IDs I add in the document transform, but you might want to not have them visible? I can see how some people will like it and some won't 😄

[mbonnin]

Yes, it's going to be controversial 😄

[mbonnin]

The ids would be in the HTTP request but not in the Kotlin model

[mbonnin]

Same for __typenane. They would be internal cache machinery unless someone add them explicitely

[mbonnin]

The Kotlin model would match 1:1 what the user typed.

[wasyl]

yeah, controversial 😄 I want my code to do what I see, really 😄 Also wouldn't that be problematic for testing, should you want to pass the invisible fields to the builders?

[mbonnin]

Ah yes, testing is an interesting topic!

[mbonnin]

I want my code to do what I see, really

That can be understood both way actually. if the compiler adds ids post codegen, then what you see (the .graphql document) is not what's in the .kt file

[wasyl]

But I wrote gql file, not the kt file. I didn't write operationHash in the gql document so why is it in the .kt file? 😛 I'm not really arguing for either option, just feels like less magic is better

[mbonnin]

Yea, #tradeoffs

[wasyl]

😄 Anyway, I managed to write my plugin and it seems to ~work, so thanks for that! I'll definitely explore the type policy annotations as well, just one question about the ordering: if I add @typePolicy, would my plugin get the transformed schema already? I don't think so, so can I use @typePolicy for most of the fields and a plugin for the remaining ones?

[mbonnin]

I think that would work

[mbonnin]

ExecutableDocumentTransform gets the 'final' schema, after any SchemaDocumentTransform

[wasyl]

oh okay. Well anyway thanks a lot for the help! Even though it took me a while, it's great that there exists mechanism to hook into and do this stuff 🙇

[mbonnin]

Nice! I wrote this as a reminder for us to improve the apollo-ast APIs