Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

feat: Improve support for nested fragments #309

Open
wants to merge 3 commits into
base: beta
Choose a base branch
from

Conversation

budde377
Copy link
Contributor

@budde377 budde377 commented May 11, 2021

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 budde377 force-pushed the fix-bad-extension branch from 4fbce42 to 89f8dfa Compare May 11, 2021 22:19
@budde377 budde377 changed the title feat: Add test to reproduce error feat: Improve support for nested fragments May 11, 2021
@budde377 budde377 force-pushed the fix-bad-extension branch from 89f8dfa to c3cc254 Compare May 11, 2021 22:22
@budde377 budde377 force-pushed the fix-bad-extension branch from c3cc254 to e9ca908 Compare May 11, 2021 22:24
@budde377 budde377 force-pushed the fix-bad-extension branch from 2d98783 to 05fbbd0 Compare May 12, 2021 06:49
@vasilich6107
Copy link
Collaborator

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

@budde377
Copy link
Contributor Author

budde377 commented May 16, 2021 via email

@vasilich6107
Copy link
Collaborator

@budde377 ok
tag me when the PR will be ready

@budde377
Copy link
Contributor Author

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

@mvarendorff
Copy link

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!

@budde377
Copy link
Contributor Author

budde377 commented Aug 29, 2021 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants