quick question, I don't have a minimal repro but t...

# apollo-kotlin

[wasyl]

quick question, I don't have a minimal repro but there's a chance I encountered a bug in how @include directive is handled: when I have two fragments on the same type, and I @include(if: false) one of them, then the whole query doesn't return from the cache. Quick outline:

query Foo(doInclude: Boolean!) {
  foo {
    bar {
      ...firstFragmentOnBar
      ...secondFragmentOnBar
      ...thirdFragmentOnBar@include(if: doInclude)
    }
  }
}

fragment firstFragmentOnBar on Bar {
  fieldforFirstFragment { ... }
}

fragment secondFragmentOnBar on Bar {
  fieldforSecondFragment { ... }
}

fragment thirdFragmentOnBar on Bar {
  fieldforThirdFragment { ... }
}

with the above when I observe Foo(false) from the cache, and another query refreshes it, then the first cache-only query doesn't emit proper data. I was pretty confused since I did see data in the normalized cache, there was no error or anything. I feel it's a bug because when I move the @include inside the fragment, it works as expected

query Foo(doInclude: Boolean!) {
  foo {
    bar {
      ...firstFragmentOnBar
      ...secondFragmentOnBar
      ...thirdFragmentOnBar
    }
  }
}

fragment firstFragmentOnBar on Bar {
  fieldforFirstFragment { ... }
}

fragment secondFragmentOnBar on Bar {
  fieldforSecondFragment { ... }
}

fragment thirdFragmentOnBar on Bar {
  fieldforThirdFragment@include(if: doInclude) { ... }
}

Am I wrong assuming the first variant is correct and should work? Or perhaps there's a subtle difference that makes the two not equivalent? Please let me know if you need more details and I'll try making a self-contained repro 🙂

[bod]

hmm no I can't think of a good reason it shouldn't work, it looks like a bug

[wasyl]

I definitely see a different behavior so I suppose something is happening 🙂 I opened an issue here: https://github.com/apollographql/apollo-kotlin/issues/5338, it's definitely low priority because there exists a trivial workaround. For now I'm leaving it but if you need a more specific repro let me know 🙂

[bod]

thanks a lot for the issue! 👍

[bod]

it may be interesting to try with v4 as there was recently a bug fix related to cache/fragment/include

[wasyl]

Yep v4 seems to work 🎉

[bod]

ah-ah! So probably the same issue. You may try with the beta.1 (which didn't have the fix) to confirm 🙂

[wasyl]

Btw is it correct migration? I want to keep using ApolloCall 🤔

[wasyl]

And beta.1 fails 🙂

[bod]

And beta.1 fails

Ok thanks for confirming 🙏

[bod]

about ApolloCall why do you now need the apply?

[bod]

oh I see storePartialResponses isn't generic, I guess we changed that 😅 No wait it is

[wasyl]

image.png

[wasyl]

but something's off 👀 It's emitCacheMisses that causes the failure

[wasyl]

but it should now be removed so it's all good 👍

[wasyl]

It doesn't use addExecutionContext so it doesn't return T 🙂

[Stylianos Gakis]

Hmm what's this apolloFactory createApolloCall etc 👀?
Are these your own apis over Apollo Kotlin?

[bod]

> but it should now be removed so it's all good
ooh yeah makes sense.

Heads up if you haven't looked at v4 before. Migration guide is here. The main change is about error handling, where we've been avoiding throwing exceptions. You can opt-in to the old v3 behavior with a configuration flag.

Also in the IDE plugin there's a migration tool, to automate a few things.

[wasyl]

Ah yes, I didn't notice it's our wrapper, but it's a simple apolloClient.query(operation). In any case I was confused with wrapper types and thought they changed in v4 but I was looking at old code 😄 It's just the emitCacheMisses inferred type in the end

[wasyl]

Thanks for the heads up, I'm aware there are some changes in the error handling, but I didn't realize it's beta time already! I think it's a good time to try the migration and report any bugs 🙂

[bod]

yes please!! 🙏 🙂

[bod]

last heads up - we're still thinking of error handling in v4, so things may still evolve