Skip to content

Make ApolloStore aware of the client CustomScalarAdapters #122

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

Open
Tracked by #2331
glung opened this issue Jan 29, 2022 · 5 comments
Open
Tracked by #2331

Make ApolloStore aware of the client CustomScalarAdapters #122

glung opened this issue Jan 29, 2022 · 5 comments
Assignees

Comments

@glung
Copy link

glung commented Jan 29, 2022

Summary

When using custom scalars, apolloStore#readOperations and apolloStore#writeOperations may throw an IllegalStateException.

This is more an API change request than a real bug (see below)

Version
3.0.0

Description

This is because the custom scalar adapters known to the client, are not known by the store. The client passes down their custom scalar adapters when writing to the store. When accessing directly the store, the custom scalar adapters are not known (it's an optional parameter of readOperation & writeOperation).

  • should the store have access to the scalar adapter?
  • should an extension function be added to the client to access the store with the required custom scalar adapters?

Below are code snippets:

appoloClient.apolloStore.readOperation(operation = query)

-> Can't map GraphQL type: `${customScalar.name}` to: `${customScalar.className}`. Did you forget to add a CustomScalarAdapter?

instead

    appoloClient.apolloStore.readOperation(
        operation = query,
        customScalarAdapters = appoloClient.customScalarAdapters
    )
@BoD
Copy link
Collaborator

BoD commented Feb 1, 2022

Hi! I think it was probably on purpose that the store and client are kind of independent / well isolated as a general good practice, but it's true that the use of custom scalars may not be the most intuitive.

An extension on the client could be something like this,

fun ApolloClient.readFromStore(operation: Operation) = apolloStore.readOperation(operation, customScalarAdapters)

but that would be the equivalent of: executing the query with a CacheOnly fetchPolicy, so I'm not sure it would be very useful?

@glung
Copy link
Author

glung commented Feb 2, 2022

but that would be the equivalent of: executing the query with a CacheOnly fetchPolicy, so I'm not sure it would be very useful?

Good point but there is no equivalent in the client's API for writeOperation.

About the API, it's was indeed unexpected for me that the store attached to the client would not know about the custom scalars adapters for the data it's effectively storing.

Other alternative options:

  • Maybe if the customScalarAdatpers parameter of read|writeOperation were not optional, I would clear some ambiguity (?).
  • Mention it in the doc store/#reading-operation-data

@martinbonnin
Copy link
Contributor

It makes sense to store the scalar adapters in ApolloStore. I think the main issue is initialization order since typically the ApolloStore is created before any call to addCustomScalarAdapter is made. Ideally, we would move the ApolloStore construction to ApolloClient.Builder.build() and pass it the adapters at that time. But since apollo-runtime doesn't know anything about the cache, we'd need to find a generic way to do this.

In all cases, updating the doc sounds like a good thing to do 👍

@martinbonnin martinbonnin changed the title ApolloStore may throw an exception when reading the cache if custom scalar adapters are used Make ApolloStore aware of the client CustomScalarAdapters Feb 7, 2022
@martinbonnin
Copy link
Contributor

Doc has been updated.

I'm keeping this issue open with a new title to keep track of the API change request. Most likely for the next major version

@martinbonnin
Copy link
Contributor

We have the same issue with the Dispatcher. See this kotlinlang thread

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

No branches or pull requests

3 participants