AniTrend / retrofit-graphql

Retrofit converter which uses annotations to inject .graphql query or mutation files into a request body, along with any variables.
https://anitrend.github.io/retrofit-graphql/
Apache License 2.0
108 stars 19 forks source link

Allow for Fragments to be defined in their own files. #9

Closed eschlenz closed 5 years ago

eschlenz commented 5 years ago

Feature Information

I don't currently see a way to define GraphQL Fragments in their own files, and use (import) them within Queries. This would be great to have, as it can significantly cut down on query object/property duplication. This is a feature that Apollo has, but appears to be missing from this tool.

I am offering to take a stab at adding this feature via PR, if your team sees the value.

Solution Information

Having only been a user of your library, my (probably naive) idea to solve this would be to simply introduce a new sibling folder to the Mutation and Query folders, called Fragment.

It seems like the flow of logic would be to first check whether the Fragment is defined within the Query file first. If not, fallback to looking for the Fragment in the Fragment folder. The contents would be read, and appended to the original Query String before sending it to the server.

This might require a level of introspection into the Query that doesn't exist yet. I don't believe you guys currently validate that a Query has all of its dependent Fragments defined; letting the server handle that instead (correct?). So in order to avoid opening the door to really complicated query parsing logic (into structured data), perhaps the dependent fragments can be located via simple regex search? A similar strategy can be used to determine whether the Fragment already exists in the Query file, or if the logic should check the Fragment folder.

It seems really straight forward, but I fully acknowledge that I may be missing something. Thoughts?

An Example Scenario

Directory structure:

assets
  Mutation
  Query
    SomeQuery.graphql
  Fragment
    ObjectAFragment.graphql
    ObjectBFragment.graphql

SomeQuery.graphql:

query SomeQuery($id: ID!) {
  objectA(id: $id) {
      ... ObjectAFragment
  }
  objectB(id: $id) {
      ... ObjectBFragment
  }
}

ObjectAFragment.graphql:

fragment ObjectAFragment on ObjectA {
  id
  name
}

ObjectBFragment.graphql:

fragment ObjectBFragment on ObjectB {
  id
  name
}

Additional context

We recently switched from using Apollo to this library, and have loved it so far. We are just at the very beginning of migrating API calls over to use this library. And we have a ton of queries, and heavily use fragments that are separated out into their own files for reuse. We'd love to be able to contribute back!

wax911 commented 5 years ago

Thank you for trying this library out, and great suggestion! I have been thinking of something similar for a while now but not at this level of detail especially due to fragment duplication. Would really appreciate a PR towards this :100:

Currently the project does not have any introspection or validating mechanisms, just simple a file name resolver. I am in favour of your suggested solution, contributions are always welcome! :star:

eschlenz commented 5 years ago

Sounds good! I'll get started on this. Thanks!

eschlenz commented 5 years ago

PR created :)

https://github.com/AniTrend/retrofit-graphql/pull/10

wax911 commented 5 years ago

Just reviewed the changes and really looks amazing, super happy about the tests :open_mouth:! I will merge and release the new feature under v1.0.0-alpha01 in case you might want to make some major API changes I hope this is fine :rocket:

Thank you very much :smiley:

eschlenz commented 5 years ago

No problem. Happy to help!