Remove CustomScalarAdapters argument from ApolloStore methods

[BoD]

Fix for #122

[martinbonnin]

The repetition of customScalarAdapter is a bit unfortunate. One option would be .apolloStore(apolloStore: (CustomScalarAdapters) -> ApolloStore), and remove .normalizedCache(). There is a bit of brain muscle around this already so maybe not the best idea. Just a suggestion.

[BoD]

The issue is that it would only work if you call .apolloStore after any call to .customScalarAdapters / .addCustomScalarAdapter. Agreed the current API is not super ideal though. Hopefully using CustomScalarAdapters is not too common anymore since adapters can be configured via Gradle and now via schema extensions (by the way - should we deprecate it in v5?)

[martinbonnin]

The issue is that it would only work if you call .apolloStore after any call to .customScalarAdapters / .addCustomScalarAdapter.

Hence the lambda. You would not pass the ApolloStore directly but a factory of ApolloStore that is called during build(), once the adapters are known.

Hopefully using CustomScalarAdapters is not too common anymore since adapters can be configured via Gradle and now via schema extensions (by the way - should we deprecate it in v5?)

Been thinking about that but I think we're definitely not there yet. Plus there might be good use cases for a runtime adapter if it needs some context that is only available at runtime.

[BoD]

Hence the lambda. You would not pass the ApolloStore directly but a factory of ApolloStore that is called during build(), once the adapters are known.

Ooh I see! But .build() is in runtime which doesn't know ApolloStore.

[martinbonnin]

The lambda could be wrapped in another one that returns an interceptor that holds the ApolloStore. It means the user does not control the order in which the cache interceptor is added but TBH feels like this was an error to have this user-controllable in the first place.

apollo-runtime could have Builder.storeInterceptor(storeInterceptor: ApolloInterceptor), Builder.apqInterceptor(apqInterceptor: ApolloInterceptor). It means the user won't be able to introduce new interceptors between those two but maybe it's for the best? Not 100% convinced but we have definitely had issues there in our API before so might be something to consider.

[martinbonnin]

Pull request there: https://github.com/apollographql/apollo-kotlin/pull/6455/files

As you can see, there's no lambda parameter to pass the CustomScalarAdapters because I was thinking we could get them from the ApolloClient:

val ApolloClient.apolloStore: ApolloStore
  get() {
    val lowLevelStore = interceptors.firstOrNull { it is CacheInterceptor }?.let {
      (it as CacheInterceptor).store
    } ?: error("no cache configured") 
    return ApolloStore(customScalarAdapters, lowLevelStore)
  }

This way the ApolloClient is still the source of truth for the adapters and we create a small bridge wrapper that connects the ApolloClient and the "thing that used to be called ApolloStore". It allocates a tiny bit but TBH this is probably nothing compared to any other ApolloStore call.

[BoD]

Yes I think that works! Let me try to use it so we can see what it looks like 👀

[BoD]

All good! Now... do we think it's OK to have it in the 4.x branch too?

[martinbonnin]

It's a behaviour change but if advertise it well, it's easy enough to update. + will ease the 5.0 migration. I'm good to have it in the 4.x branch

[BoD]

All right! 🚀