apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.36k stars 2.66k forks source link

Support for custom scalars #585

Closed Poincare closed 6 years ago

Poincare commented 8 years ago

After speaking with @glasser, I noticed that we don't currently have great support for custom scalar types on Apollo Client. I understand that these are probably pretty difficult to support without having the schema on the client but we could make it possible to add some custom support for scalar serialization on the client.

Although custom scalar types are certainly important for lots of applications, considering some of the other features and fixes we have to build, this is probably not hugely important at the moment. (I could definitely be wrong on this.)

theodorDiaconu commented 6 years ago

The main issue this has is the fact that your client has to know the full schema and you need deserialisation/serialisation logic on the client.

Wouldn't it be easy if we do something like:

const DateScalar = { parseValue, serialize }
const ScalarMap = {
   User: {
       createdAt: DateScalar
   }
}

Now we need to hook into all responses: https://www.apollographql.com/docs/react/advanced/network-layer.html#linkAfterware https://github.com/apollographql/apollo-client/blob/master/Upgrade.md#afterware-data-manipulation

And have a recursive scalar parser based on __typename, and you could even hook into mutations deep in the GraphQLOperation object, to apply these parsers there as well.

Most of the cases when we need this is for Date, I did not see the need for something else... yet.

If time permits I'll roll this out next week, unless someone takes up the challenge and does it faster!

theodorDiaconu commented 6 years ago

It's done: https://github.com/cult-of-coders/apollo-client-transformers

ShockiTV commented 6 years ago

Link is nice place, but I feel like this will make cache unserialisable and break it's persist/restore coop with for example apollo-cache-persist (which have option to turn json serialisation off - but who knows which storage's require it etc).

On cache level, there are already operations per __typename so for me it sounds like better place where do the manipulation.

Akryum commented 6 years ago

Maybe it should also provide a way to serialize Dates back to numbers for parameters and inputs. In the cache there would only be Dates then.

fbartho commented 6 years ago

@theodorDiaconu -- I love that you made this as a link. I totally concur with @ShockiTV, however -- I don't believe right now, that we can use a Link to transform responses without them being serialized back to the memory cache, which could easily explode.

I believe we need a new feature in Apollo-Client where we Scalars are inflated at the last second, before returning the response to consumer-code.

I think the cache / apollo-cache-persist should have Scalars de-normalized back to their "on-the-wire" format.

theodorDiaconu commented 6 years ago

You guys are right. The only options left are to hack the .query() from apollo-client, and maybe .readQuery() or find a way so cache persisting/restore uses these transformers.

On 27 Jun 2018, at 9:59 PM, Frederic Barthelemy notifications@github.com wrote:

@theodorDiaconu -- I love that you made this as a link. I totally concur with @ShockiTV, however -- I don't believe right now, that we can use a Link to transform responses without them being serialized back to the memory cache, which could easily explode.

I believe we need a new feature in Apollo-Client where we Scalars are inflated at the last second, before returning the response to consumer-code.

I think the cache / apollo-cache-persist should have Scalars de-normalized back to their "on-the-wire" format.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

micimize commented 6 years ago

@ShockiTV not all storage engines support serialize: false - window.localStorage.setItem('foo', { bar: 'bar' }) sets foo to "[object Object]". It seems more like an "if you know what you're doing" option, like if you're using localForage and want to let it handle json transcoding.

@fbartho Deferring deserialization to data injection time is suboptimal and will be non-negligibly expensive in many situations. Even if there is currently some problem with storing non-json in the cache, I think that problem would need to be solved before we have can have "true" scalar support.

@theodorDiaconu you're very right, my solution is heavy handed and coupled to the rest of my architecture / toolchain. But it also leaves the door open for deserializing fragments and documents into more sophisticated types, like sorted collections, etc. I definitely think a mature ecosystem would have solutions in both styles.

hwillson commented 6 years ago

To help provide a more clear separation between feature requests / discussions and bugs, and to help clean up the feature request / discussion backlog, Apollo Client feature requests / discussions are now being managed under the https://github.com/apollographql/apollo-feature-requests repository.

Migrated to: https://github.com/apollographql/apollo-feature-requests/issues/2