https://kotlinlang.org logo
#apollo-kotlin
Title
# apollo-kotlin
p

Paddy O'Brien

08/03/2022, 2:14 PM
Working on a V2 to V3 upgrade and running into an issue with changes in code generation. We have a Union over a number of types that all have the same shape
Copy code
{
    __typename
    name: String
    value: Type
}
Where
Type
varies from union member to union member. In V2
Type
was generated as a dataclass. Admitedly the types are
Any
and do no preserve the graphql types. Is there a way to trigger this behaviour in v3? or do we need to rewrite thew query using aliases
Fragment
Copy code
fragment PropertyValue on PropertyValue {
    __typename
    ... on PropertyValueBoolean {
        value
    }
    ... on PropertyValueFloat {
        value
    }
    ... on PropertyValueInt {
        value
    }
    ... on PropertyValueJson {
        value
    }
    ... on PropertyValueString {
        value
    }
}
Generated model
Copy code
data class PropertyValue(
  val __typename: String = "PropertyValue",
  val asPropertyValueBoolean: AsPropertyValueBoolean?,
  val asPropertyValueFloat: AsPropertyValueFloat?,
  val asPropertyValueInt: AsPropertyValueInt?,
  val asPropertyValueJson: AsPropertyValueJson?,
  val asPropertyValueString: AsPropertyValueString?
m

mbonnin

08/03/2022, 10:50 PM
Hi 👋 Can you ellaborate what you would like the generated code to look like?
Or wait, looks like what you pasted is actually 2.x, right ?
Just tried it, looks like you're bumping in this error:
Copy code
`value` cannot be merged with `value`: they have different shapes.
I think this is expected? This is that part of the spec: https://spec.graphql.org/draft/#sel-GALRFFfCAACGBssG
Copy code
However, the field responses must be shapes which can be merged. For example, scalar values must not differ.
I'm curious how come this has worked before though 🤔
It's pretty late here and I'll log off soon but feel free to follow up and I'll look into it first thing tomorrow morning
p

Paddy O'Brien

08/04/2022, 2:56 PM
Yeah I pasted the 2.x generated code. The scalar types are not being preserved in 2.x. Underneath the
value
property is just
Any
I believe.
m

mbonnin

08/04/2022, 2:57 PM
What type is the
value
field in the different
PropertyValueXYZ
types?
p

Paddy O'Brien

08/04/2022, 2:57 PM
The docs you point to have the example as querying against an Interface. Unions would exhibit different behaviour I think though the docs aren’t particular on this point
2 secs gonna have to switch branches
Apollogies, ran into build issues when switching branches and then meeting happened. This is the generated code under V2.
I was wrong, only some types for
value
are
Any
m

mbonnin

08/04/2022, 5:30 PM
I'm still curious how come this works...
I made a quick test with
graphql-js
which is the reference implementation:
Copy code
var schema = buildSchema(`
  type Query {
    foo: PropertyValue
  }
  union PropertyValue = PropertyValueBoolean | PropertyValueInt

  type PropertyValueBoolean {
    name: String!
    value: Boolean
  }
  type PropertyValueInt {
    name: String!
    value: Int
  }
  `);

  var document = parse(`
  query GetFoo {
    foo {
      ...PropertyValue
    }
  }
  fragment PropertyValue on PropertyValue {
    __typename
    ... on PropertyValueBoolean {
        value
    }
    ... on PropertyValueInt {
        value
    }
  }
  `)

  var errors = validate(schema, document)
This fails with
Copy code
Fields "value" conflict because they return conflicting types "Boolean" and "Int". Use different aliases on the fields to fetch both if this was intentional.
What framework are you using on the backend?
p

Paddy O'Brien

08/04/2022, 5:53 PM
I dont even know, something on rails
m

mbonnin

08/04/2022, 5:55 PM
Double checking: the problem you're having is that error at compile time, right?
Copy code
`value` cannot be merged with `value`: they have different shapes.
p

Paddy O'Brien

08/04/2022, 5:55 PM
yeah, happens on 3x not on 2x
m

mbonnin

08/04/2022, 5:55 PM
So I think that's the expected behaviour and it working before was not compliant with the spec
p

Paddy O'Brien

08/04/2022, 5:56 PM
it’s not a huge issue, was just wondering if there was a way to restore the old behaviour.
m

mbonnin

08/04/2022, 5:57 PM
We aim at spec compliance so the only way to do that would be through something like override flags that disable some validations
But TBH I'm quite reluctant to do this as this will be a pain to maintain
p

Paddy O'Brien

08/04/2022, 5:58 PM
It does seem strange to me that unions would require congruent types in the event of name collisions. Interfaces make sense to me but a union could be made of any types anywhere in the schema and some names like value would be prone to collisions. I also cant think of another way to to heterogenous collections
I can alias the fields. The old behaviour just made the call site nicer for how we used it
m

mbonnin

08/04/2022, 5:59 PM
p

Paddy O'Brien

08/04/2022, 6:04 PM
yeah, reading that now
m

mbonnin

08/04/2022, 6:08 PM
If you're willing to contribute a flag to disable that specific validation, I think we could merge that. Ping @bod any thoughts?
b

bod

08/04/2022, 6:13 PM
agreed, if it's "just" disabling a specific validation I don't see any major drawback 🙂
m

mbonnin

08/04/2022, 6:13 PM
"just" another Gradle config we'll have to maintain until the end of times 🙃
Hopefully the end of times comes soon 😛
p

Paddy O'Brien

08/04/2022, 6:14 PM
😄
willing yes, finding time is another issue. Will create an issue internally and dig into it when I have time lol
m

mbonnin

08/04/2022, 6:22 PM
p

Paddy O'Brien

08/04/2022, 6:27 PM
Thanks!
m

mbonnin

08/30/2022, 6:39 PM
Hi @Paddy O'Brien 👋`pt2121` contributed a change to do exactly what you want: https://github.com/apollographql/apollo-kotlin/pull/4342
(disabling that validation)
Not sure if it's a happy coincidence or if you happen to work with the same schema 🙂
In all cases, let us know how that sounds for you!
p

Paddy O'Brien

08/30/2022, 6:54 PM
Happy coincidence 😄
m

mbonnin

08/30/2022, 7:03 PM
It's a small world 🙂 This has litterally never been an issue in years
p

Paddy O'Brien

08/30/2022, 7:35 PM
Might not be a coincidence, it wasn’t validated in 2.0, there are some libs that require upgrading to get M1 support but those versions also require Gradle 7. Given the increasing popularity of M1 macs there may be a more Gradle upgrades happening
41 Views