artemis icon indicating copy to clipboard operation
artemis copied to clipboard

feat: Improve support for nested fragments

Open budde377 opened this issue 3 years ago • 6 comments

Make sure you're opening this pull request pointing to beta branch!

What does this PR do/solve?

This PR improves fragment support.

Given the schema:


type Query { 
  interface: InterfaceA
  union: UnionA
} 

interface InterfaceA {
  s: String
  i: Int
}
union UnionA = ImplementationA | ImplementationB
type ImplementationA implements InterfaceA {
  s: String
  i: Int
  b: Boolean
}
type ImplementationB implements InterfaceA {
  s: String
  i: Int
  i2: Int
}

and the query

fragment InterfaceFragment on InterfaceA { 
  s
  i
  ...on ImplementationA {
    b
  }
  ...on ImplementationB {
    i2
  }
}
fragment UnionFragment on UnionA { 
  ...on ImplementationA {
    b
  }
  ...on ImplementationB {
    s
  }

}
query some_query { 
  interface {
    ...InterfaceFragment
  }
  union {
    ...UnionFragment
  }
}

This is an example of what Artemis currently generates:


@JsonSerializable(explicitToJson: true)
class SomeQuery$Query$InterfaceA extends FileSystemEntryMixin
    with EquatableMixin {
  SomeQuery$Query$InterfaceA();
  factory SomeQuery$Query$InterfaceA.fromJson(Map<String, dynamic> json) =>
      _$SomeQuery$Query$InterfaceAFromJson(json);
  @override
  List<Object?> get props => [s, i];
  Map<String, dynamic> toJson() => _$SomeQuery$Query$InterfaceAToJson(this);
}

The code above is not valid Dart code.

Instead, I believe it should be:


@JsonSerializable(explicitToJson: true)
class SomeQuery$Query$InterfaceA extends JsonSerializable
    with EquatableMixin, InterfaceFragmentMixin {
  SomeQuery$Query$InterfaceA();
  factory SomeQuery$Query$InterfaceA.fromJson(Map<String, dynamic> json) =>
      _$SomeQuery$Query$InterfaceAFromJson(json);
  @override
  List<Object?> get props => [s, i];
  Map<String, dynamic> toJson() => _$SomeQuery$Query$InterfaceAToJson(this);
}

budde377 avatar May 11 '21 22:05 budde377

Hi, thanks for PR I’ll check it this week

vasilich6107 avatar May 16 '21 14:05 vasilich6107

You’re welcome. I think this needs some additional work on handling fragment spreads to work properly though, so let me have a look at that.

On Sun, 16 May 2021 at 15:35, Vasiliy Ditsyak @.***> wrote:

Hi, thanks for PR I’ll check it this week

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/comigor/artemis/pull/309#issuecomment-841825968, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2UFS3P46BNWZBQXBCV3LLTN7J27ANCNFSM44XDC5ZQ .

budde377 avatar May 16 '21 15:05 budde377

@budde377 ok tag me when the PR will be ready

vasilich6107 avatar May 20 '21 14:05 vasilich6107

@vasilich6107.

I think the changes needed are pretty big, so to test my hypothesis I created this project: https://pub.dev/packages/graphql_codegen

Here I create "interfaces" for fragments and fields, instead of mixins, and create classes for operations and selection sets. I'll be using that package instead since I'm using the graphql_flutter client, but feel free to take anything you can use.

budde377 avatar May 23 '21 10:05 budde377

Hi, sorry for this half necro. I was wondering whether there are there any plans to integrate this into artemis (from either of both parties involved in this thread so far) as I hit several issues with nested fragments recently as well and got excited to see this being worked on.

If there are still changes required, please let me know if I can look into it and where I should start!

mvarendorff avatar Aug 23 '21 06:08 mvarendorff

I ended up creating my own package with better fragment support:

https://pub.dev/packages/graphql_codegen

Here's a post outlining the motivation: https://budde377.medium.com/structure-your-flutter-graphql-apps-717ab9e46a5d

On Mon, Aug 23, 2021 at 8:49 AM geisterfurz007 @.***> wrote:

Hi, sorry for this half necro. I was wondering whether there are there any plans to integrate this into artemis (from either of both parties involved in this thread so far) as I hit several issues with nested fragments recently as well and got excited to see this being worked on.

If there are still changes required, please let me know if I can look into it and where I should start!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/comigor/artemis/pull/309#issuecomment-903490234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2UFSZU6EWPGFTCXKOW7RTT6HVPTANCNFSM44XDC5ZQ .

budde377 avatar Aug 29 '21 17:08 budde377