Skip to content
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

Make GraphQLQuery JsonSerializable #62

Open
smkhalsa opened this issue Dec 29, 2019 · 14 comments
Open

Make GraphQLQuery JsonSerializable #62

smkhalsa opened this issue Dec 29, 2019 · 14 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@smkhalsa
Copy link

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 comigor added enhancement New feature or request good first issue Good for newcomers labels Jan 2, 2020
@comigor
Copy link
Owner

comigor commented Jan 2, 2020

Hello @smkhalsa!

That surely seems doable and makes sense!

@smkhalsa
Copy link
Author

smkhalsa commented Jan 5, 2020

@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
Copy link
Owner

comigor commented Jan 6, 2020

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
Copy link
Contributor

klavs commented Jan 7, 2020

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
Copy link
Owner

comigor commented Jan 7, 2020

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
Copy link
Contributor

klavs commented Jan 7, 2020

@smkhalsa
Copy link
Author

smkhalsa commented Jan 9, 2020

@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
Copy link
Owner

comigor commented Jan 9, 2020

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
Copy link
Author

smkhalsa commented Jan 9, 2020

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
Copy link
Owner

comigor commented Jan 10, 2020

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
Copy link
Author

smkhalsa commented Jan 10, 2020

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
Copy link
Owner

comigor commented Jan 13, 2020

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
Copy link
Author

@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
Copy link

CosmicPangolin commented Nov 12, 2021

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants