-
Notifications
You must be signed in to change notification settings - Fork 66
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
Integrate async-graphql
#822
Comments
async-graphql
capabilities. async-graphql
@rvmelkonian, @ra0x3: I've listed some of my thoughts here. what we needFor query execution:
For schema creation:
For macros:
what
|
Context and History 📚
This originally came up in a comment on a PR here. Some discussion has been had about the pros and cons of moving away from our own
fuel-indexer-graphql-parser
package and to async-graphql.The original fuel graphql api not maintained by us made use a forked version of graphql-parser. We then added this code into our repo and we make heavy use of its types and other functionalities. This was merged in to master in this PR.
The use of the async-graphql crate came into play when we started supporting the GraphQL playground, we needed the crate in order to provide this functionality. See the merged PR for more context.
In its current state, we are using both these crates.
Why move to async-graphql? 🤔
• Async-graphql provides a lot of functionality out of the box, such as docs and schemas within the playground, multiple queries in a single request and probably much more. Full list (copied from their readme)
Currently we have to hand-roll a lot of this functionality as we are using the
fuel-indexer-graphql-parser
types.• By utilising the full potential of
async-graphql
we would reduce the size of our binaries, as we would perhaps not need to usefuel-indexer-graphql-parser
.• Probably most of the features public devs will want from the api, will be easily importable using the package.
Why not to move to async-graphql? 🔕
• The main reason we use our own types is because we process custom, user generated schemas not known at compile time. Furthermore, these schemas do not support all graphql types, because of our codegen.
• We stay in control of our own parser types and functionality, no reliance on a package for the critical piece.
The main point -> Supposedly async-graphql supports dynamic schemas see here but then @deekerno also found this issue, which suggests the opposite. This is unclear for us currently.
• We need to test this dynamic schema functionality, if it can support our needs, then this would be a very powerful addition, and would mean we would need to hand craft much of the graphql functionality already provided by
async-graphql
After the questions in this issue are answered, and the unknowns made known, we can come up with a plan about how to deprecate our old
graphq-parser
code (if we indeed choose to).Sub-issues (feel free to add more) 🏁
Dynamic Schemas
Feel free to discuss and share your thoughts @ra0x3 @deekerno
The text was updated successfully, but these errors were encountered: