Use new version of async-graphql for dynamic schema generation
#287
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adzialocha
changed the title
async-graphql for dynamic schemaasync-graphql for dynamic schema
adzialocha
changed the title
async-graphql for dynamic schemaasync-graphql for dynamic schema generation
adzialocha
requested changes
Mar 14, 2023
There was a problem hiding this comment.
I can't believe how easy it is to understand this and at the same time it is super powerful. It's really nicely done!
I've added some smaller comments in my review, some general remarks:
- You have a whole bunch of "TODO" markers in the code which partly need to be extended or discussed. My suggestion would be to remove them and rather create GitHub issues instead? Then we can continue discussion there.
- There is a lot of underscores 😆 Don't think we make use of that naming pattern so far, what was the intention here? Is this a sign of too much dead code which needs to be removed? In general I think its good to not keep dead code around at any point.
* main: Update breaking API calls for new `p2panda-rs` version (#293) Update Cargo.lock as well Use released p2panda-rs version 0.7.0 Migrate CLI from `structopt` to `clap` (#289) Increase timeout for failing materializer test Introduce requeue flag (#286) Do transactions correctly (#285) Add libp2p service and configuration (#282)
cafca
reviewed
Apr 29, 2023
Comment on lines
+27
to
+33
| move |ctx| { | ||
| FieldFuture::new(async move { | ||
| // Here we just pass up the root query parameters to be used in the fields resolver | ||
| let params = downcast_id_params(&ctx); | ||
| Ok(Some(FieldValue::owned_any(params))) | ||
| }) | ||
| }, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use https://github.com/async-graphql/async-graphql and https://github.com/smmoosavi/dynamic-graphql to implement dynamic schema handling in our graphql api.
This took a little bit of time as I had to get to grips with the new
async-graphqldynamic schema creation patterns, but I'm very happy with the result, it will be much more enjoyable (and quite quick) to expand our query api now with new features.Design decisions
async-graphqlto build all queries dynamically. It doesn't seem possible to combine "statically" defined queries with dynamic ones, so evennextArgsis defined using the dynamic APIdynamic-graphqlwhich offers macro wrappers forasync-graphql, these are then registered on the root schema and can be referenced by name in dynamic queries where object types can be definedTasks
graphqlmoduleadd some more testslet's do this in next steps 😶🌫️ we at least shouldn't have fewer tests than we did before!getit already wasdynamic-schemapublished or remove it from this PRNext steps
📋 Checklist
CHANGELOG.md