Skip to content
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

Feature: merge object definitions #182

Open
trsh opened this issue May 16, 2018 · 12 comments
Open

Feature: merge object definitions #182

trsh opened this issue May 16, 2018 · 12 comments
Labels
enhancement Improvement of existing features or bugfix

Comments

@trsh
Copy link

trsh commented May 16, 2018

File one

graphql_object!(QueryRoot: Pool<ConnectionManager<PgConnection>> |&self| {
    field constant(&executor, id: String) -> FieldResult<Option<Constant>> {
        ......
    }
});

File two

graphql_object!(QueryRoot: Pool<ConnectionManager<PgConnection>> |&self| {
    field ssssss(&executor) -> FieldResult<Vec<Constant>> {
        .....
    }
});

Combining file


use juniper::RootNode;

pub struct QueryRoot;
pub struct MutationRoot;

pub type Schema = RootNode<'static, QueryRoot, MutationRoot>;

pub fn create_schema() -> Schema {
    Schema::new(QueryRoot {}, MutationRoot {})
}

Obviously this aint working because of conflicting implementation of QueryRoot. Any suggestion how to go around this? I do not really want a super huge Schema file with all objects in it ! :/

@trsh trsh changed the title How build QueryRoot in multiple files How build QueryRoot in multiple files? May 16, 2018
@trsh
Copy link
Author

trsh commented May 16, 2018

Nesting works for me, but adds complexity to the ql. So any suggestions till appreciated.

@piperRyan
Copy link
Contributor

piperRyan commented Jun 17, 2018

@trsh Typically you only have one root then have nested objects within it it, that is,the graphql schema must contains a single graphql object, it cannot be an array of objects and so forth. This being said you can get around this a bit if you need to by setting up a proxy via nginx or apache so that each namespace points to a different graphql instance (but I kind of warn against this), but are you sure you just don't want.

file_1.rs

use file_2::File2Obj; 

graphql_object!(QueryRoot: Pool<ConnectionManager<PgConnection>> |&self| {
    field constant(&executor, id: String) -> FieldResult<Option<Constant>> {
        ......
    }

   field file_2(&executor) -> FieldResult<File2Obj> {
         // logic to generate File2Obj
   }
});

file_2.rs

graphql_object!(File2Obj: Pool<ConnectionManager<PgConnection>> |&self| {
    field ssssss(&executor) -> FieldResult<Vec<Constant>> {
        .....
    }
});

Or if that isn't the case can you specify what you are trying to achieve via the query and I can give a suggestion on how to do so.

@trsh
Copy link
Author

trsh commented Jun 18, 2018

Nesting worked out fine form me

@trsh trsh closed this as completed Jun 18, 2018
@theduke theduke reopened this Jun 20, 2018
@theduke
Copy link
Member

theduke commented Jun 20, 2018

I'd still like to keep this open as a feature request because I want the same functionality of merging two separate object definitions into one.

@theduke theduke added the enhancement Improvement of existing features or bugfix label Jun 20, 2018
@theduke theduke changed the title How build QueryRoot in multiple files? Feature: merge object definitions Jun 20, 2018
@trsh
Copy link
Author

trsh commented Jun 25, 2018

@theduke cool

@Darkeye9
Copy link

I also think this should be supported! I decided to use nesting, but this could be a very neat feature to have

+1

@Swoorup
Copy link

Swoorup commented Jan 6, 2019

Same here, came looking for laying out multiple graphql endpoints to different files by business logic rather than dump everything to one file.

@jgillich
Copy link
Contributor

Another one. I'm planning to build a fairly large API with lots of fields. All data objects have a parent object for categorization. Something like that:

categoryOne {
  objectOne { ...data }
  objectTwo { ...data }
}
categoryTwo {
  objectOne { ...data}
}

And each category can have 10+ fields, so I don't want to have them all in the same file.

I could try to implement this, but my Rust skills are a bit rusty (haha get it)(sorry) and I'm not familiar with the code base nor do I remember much about how macros work, so I'm not sure if I should. 😄

@theduke
Copy link
Member

theduke commented Jan 28, 2019

This could be an extension to the GraphQLObject custom derive.

It would look something like this:

use objects::{ObjectOne, ObjectTwo, ...};

#[derive(juniper::GraphQLObject)]
struct CategoryOne {
  #[juniper(merge)]
  obj1: ObjectOneQuery,
  #[juniper(merge)]
  obj2: ObjectTwoQuery,
}

The custom derive code would need to build the code for impl GraphQLType for Query

The GraphQLType docs show a pretty good example of how the trait works.

The problem here is that we wouldn't know the structure of the merged types at compile time, so the impl would have to dynamically compute the info or the field resolver logic.

  • GraphQLType::meta(): this would have to call ::meta() for each of the merged types and merge the result together. This is also the only time we could warn about field name conflicts by panicking.

  • GraphQLType::resolve_field() would then have to decide which child types resolve_field() must be called. For performance, there should be a lookup table here that is only created once.

The code for the custom derive is here: https://github.com/graphql-rust/juniper/blob/master/juniper_codegen/src/derive_object.rs.

The code would need to be extended to detect the merge(...) part of the attribute and then generate the appropriate code.

I'm happy to give more guidance here or on Gitter if you want to tackle it, @jgillich .

@jgillich
Copy link
Contributor

@theduke Thank you, this is great! I'm not sure when I'll get to this, but I'll let you know in case I need any help.

@vladinator1000
Copy link

vladinator1000 commented Jul 12, 2019

I'd love to be able to do something like this to avoid cramming everything in one file in big apps. Happy to help out on a PR anytime.
image

@vladinator1000
Copy link

I tweeted to see if anyone is interested in this and looks like people would love to have it: https://twitter.com/vladkodmc/status/1162709927993008128?s=20

I'm looking to open a proof of concept PR for this soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix
Projects
None yet
Development

No branches or pull requests

7 participants