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

Proposal: Solution to reduce/control cargo check times as the codebase grows #1023

Closed
Gricha opened this issue Aug 13, 2022 · 4 comments
Closed
Labels
enhancement New feature or request Stale

Comments

@Gricha
Copy link
Contributor

Gricha commented Aug 13, 2022

Hey! We started hitting pretty long cargo check times as our project grows. I've seen previous thread
about this (783) but regardless of short term fixes, we are
expecting our issues to continue to grow as our project grows.

Ideally we would want to be able to split our graph into multiple libraries and there exists some
way of doing this already:

  • Splitting it by domains where parts of graph are fully isolated from the other parts
  • Using MergedObject to hoist up parts of the objects from downstream dependency - this is the preferable method for us.

The first one doesn't really work for us since majority of our graph is fairly interconnected as we
heavily optimize for frontend being able to compose UI pretty easily. Meaning that we end up with a lot
of nodes referring back other nodes in the graph.

Problem

As an example of what I'd like to be able to eventually do is the following (and there's a zip file with this project attached as it probably illustrates the issue better):

  1. Have leaf library define a core part of the object:
    In leaf-lib/lib.rs
#[derive(SimpleObject)]
pub struct UserCore {
    pub name: String,
    pub age: i64,
}
  1. Have top-level add necessary connections/fields that leaf library can't:
    in main.rs
pub struct UserExtension;

#[Object]
impl UserExtension {
    // you could also just imagine this refers to a node in the graph that the leaf library can't see

    async fn added_field<'ctx>(&self, _ctx: &Context<'ctx>) -> i64 {
        1
    }
}

#[derive(MergedObject)]
pub struct User(UserCore, UserExtension);
  1. PROBLEM: Let's extract another object to a library, it needs to refer to User, so it depends on our leaf-lib. For instance
    In middle-lib/lib.rs
pub struct Group;

#[Object]
impl Group {
    async fn users(&self) -> Result<Vec<UserCore>> {
        Ok(vec![])
    }
}

Notice that this object can only refer to UserCore and not User, because User is defined at the top-level as extension.

Expected behavior: What I hoped is this would generate a schema like this:

type Group {
	users: [User!]!
}

type QueryRoot {
	users: [User!]!
	groups: [Group!]!
}

type User {
	name: String!
	age: Int!
	addedField: Int!
}

schema {
	query: QueryRoot
}

but the actual schema will expose that there exist UserCore and Group refers to it:

type Group {
	users: [UserCore!]!
}

type QueryRoot {
	users: [User!]!
	groups: [Group!]!
}

type User {
	name: String!
	age: Int!
	addedField: Int!
}

type UserCore {
	name: String!
	age: Int!
}

schema {
	query: QueryRoot
}

Proposal

Now what I would propose as an enhancement is an API that is similar to MergedObject (or modify MergedObject) to help achieve the proposed. For now let me call this an ObjectExtension:

  • If an Object is used somewhere where an ObjectExtension is also used, this Object cannot be directly exposed in the schema, and instead its instances will be swapped for whatever the object is extended with.
  • The limitation of that is that we need to ensure that if FooCore is the core object and FooExtension is the Object we extend with, From<Foo> for FooExtension is true as this will allow
    dynamically construct the actual Foo

What you gain as the benefit of that is:

  • GraphQL schema that can be interconnected as you please without sacrificing frontend usability for technical limitations of the backend.
  • Ability to split backend into multiple libraries pretty easily in a way that makes schema safe and not leak any internal details.

I've attached a ZIP file with a project example that compiles and can be played with that uses MergedObject - this may illustrate my issue a little bit better, but if it doesn't, please let me know - I'm happy to explain more what I mean. I am also happy to lend a hand with the implementation of something like this if we agree to its usefulness.

The project code

test-async-graphql.zip

@Gricha Gricha added the enhancement New feature or request label Aug 13, 2022
@sunli829
Copy link
Collaborator

This looks useful, but it can be complicated to implement.

@Gricha
Copy link
Contributor Author

Gricha commented Aug 23, 2022

Yeah I agree that full proposal would be pretty tricky. I think the more minimal step towards the full solution would be to provide a flag, or a way of controlling Object to not be part of the schema. If you could write something like this:

pub struct UserCore {
  id: i64
}

#[Object]
#[graphql(is_node = false)]
impl UserCore {
  // .. some fields here
}

And then it is users responsibility to create actual Merge object, such that this part of schema is ever reachable.

Compared to original proposal it eliminates a bunch of complexity - the only thing it tries to say is "This object should never be returned by itself as part of the schema", as it unnecessarily bloats the schema and unveils unnecessarily the implementation details.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Sep 23, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 5 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale
Projects
None yet
Development

No branches or pull requests

2 participants