comigor / artemis

Build dart types from GraphQL schemas and queries
MIT License
495 stars 119 forks source link

Make GraphQLQuery JsonSerializable #62

Open smkhalsa opened 4 years ago

smkhalsa commented 4 years ago

I'm creating a GraphQL client that persists mutations when offline and runs them when the client comes back online.

I'd like to request that we make the GraphQLQuery class JsonSerializable so that we can easily persist queries for later use.

comigor commented 4 years ago

Hello @smkhalsa!

That surely seems doable and makes sense!

smkhalsa commented 4 years ago

@comigor awesome! Any ideas on how to implement this? I'm still a novice when it comes to code gen with build_runner.

It seems that the GraphQLQuery class would need to be aware of all the generated subclasses in order to parse them correctly.

comigor commented 4 years ago

You'll probably want to make all generated extensions of GraphQLQuery (like this one) to have a JsonSerializable annotation to be able to convert its fields to json.

Looking at it now, the main issue is that DocumentNode is not serializable as well, hace you'll need to open a PR on gql/ast making everything serializable as well.

klavs commented 4 years ago

Serializing the AST to a GraphQL string might be a better choice as you would be able to de-serialize / parse it even if the AST makes some incompatible changes

comigor commented 4 years ago

That's perfect, is it possible to serialize it to a String from a DocumentNode? If so, json_serializable supports fromJson and toJson options on fields.

klavs commented 4 years ago

Here's what you need: https://pub.dev/documentation/gql/latest/language/language-library.html

smkhalsa commented 4 years ago

@comigor it seems printNode & parseString referenced by @klavs will work for DocumentNode. However, it's still unclear to me how best to handle the parse function. We would need the abstract GraphQLQuery class to be able parse a JSON object into any of its subclasses (since we won't know which concrete class the object represents).

Any ideas?

comigor commented 4 years ago

Hmm, yeah, you won't be able to serialize the function... and not having reflection don't help us at all...

I thought about you creating a custom (serializable) class extending from GraphQLQuery<JsonSerializable, JsonSerializable> and create your custom logic to override the parse function to return an object based on the json argument and the query name of what you're persisting. But even so, you'll need to make your user manually configure your client with a map of operation name to output object function, something like this:

Your custom query:

@JsonSerializable()
class MySerializedQuery
    extends GraphQLQuery<JsonSerializable, JsonSerializable> {
  @override
  JsonSerializable parse(Map<String, dynamic> json) =>
      options.myConfigurationMapping[operationName](json);

  @override
  List<Object> get props => null;
}

The configuration your user'll need to configure while instantiating your client:

final client = MyGraphQLClient(
  myConfigurationMapping: <String,
      JsonSerializable Function(Map<String, dynamic>)>{
        'operation1': MyResultClass.fromJson;
      },
);

Or, as an alternative, you make your client a generator ¯\(ツ)

Does it make sense?

smkhalsa commented 4 years ago

you'll need to make your user manually configure your client with a map of operation name to output object function

Can't we just have the generator generate the GraphqQLQuery class and include the mapping you describe above?

For example, we could store the type name as a string when calling toJson and implement a concrete parse method on the generated GraphQLQuery class that maps those strings to the concrete classes.

abstract class GraphQLQuery<T, U extends JsonSerializable> extends Equatable {

  ///... other class members

  T parse(Map<String, dynamic> json) {
    switch (json) {
      case json['type'] == 'MySerializedQuery':
        return MySerializedQuery.parse(json);
      /// other class resolvers
    }
  }
}

Or, as an alternative, you make your client a generator ¯(ツ)/¯

I've thought about this, and I may go in that direction eventually (I just haven't dug into code generation yet). A fully typed generated client would definitely make for a nice dev experience.

comigor commented 4 years ago

Can't we just have the generator generate the GraphqQLQuery class and include the mapping you describe above?

But how are you going to persist and (more importantly) retrieve those queries? When you deserialize them, you need to have a way of knowing which type it belongs, and then it's best to have a single place where this logic will be present (and not scattered through all generated queries).

Does it make sense? Or I am missing something?

smkhalsa commented 4 years ago

But how are you going to persist and (more importantly) retrieve those queries?

You could generate the abstract GraphQLQuery class and include it in the generated result. It would include a parse constructor that would map to the respective subclass's parse function.

/// [generated_code]

abstract class GraphQLQuery<T, U extends JsonSerializable> extends Equatable {

  ///... other class members

  GraphQLQuery.parse(Map<String, dynamic> json) {
    switch (json) {
      case json['type'] == 'MySerializedQuery':
        return ExampleQuery.parse(json);
      /// other class resolvers
    }
  }
}

class ExampleQuery extends GraphQLQuery<Example, ExampleArguments> {

  /// other class members

  T parse(Map<String, dynamic> json) {
    /// generated parse logic for this class
  }
}

Then to deserialize, you'd simply do something like this:

final deserializedQuery = GraphQLQuery.parse([some json object])

The parse functions of the subclasses would be ignored when serializing them.

Does that make sense?

comigor commented 4 years ago

But I disagree the abstract GraphQLQuery should have this implementation, that's why I've suggested to create a new Query class, extending GraphQLQuery, and to deserialize your saved queries into it.

smkhalsa commented 4 years ago

@comigor sure, that makes sense.

For reference, built_value adds a "discriminator" that stores a string reference to the class name when serializing. This allows for deserializing even when you don't know the type.

It may be more than we want to take on at the moment, but we could also just make queries implement built_value and get this functionality for free.

CosmicPangolin commented 2 years ago

@comigor

Is there any chance this package gets a built_value implementation at this point? I'd love to use it for personal and agency projects but we're committed to the builder pattern, immutability, and out-of-the-box discriminator + serialization (including the optional plugins for things like DateTime). We find the immutability and patterning incredibly valuable so we even choose built_value and built_collection for lots of things beyond data models in our architecture, like bloc states...compared to json_serializable, the output is strictly more sound and robust data structures.