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

Eager-load multiple relationships through @with #1132

Open
chescos opened this issue Jan 11, 2020 · 6 comments · May be fixed by #2525
Open

Eager-load multiple relationships through @with #1132

chescos opened this issue Jan 11, 2020 · 6 comments · May be fixed by #2525
Labels
enhancement A feature or improvement

Comments

@chescos
Copy link
Collaborator

chescos commented Jan 11, 2020

Is your feature proposal related to a problem? Please describe.

It seems that it is currently not possible to eager-load multiple relationships through the @with directive.

Describe the solution you'd like

I think it should either be possible to use the @with directive more than once on a single field, or the @with directive should accept an array of relationships.

  • Using the @with directive multiple times: I'm not sure if it's currently possible to use any directive more than once on a single field. Is this being prevented for some reason?
  • Accepting an array of relations through the @with directive: The question is how to handle scopes then, should they be part of the relations array (e.g relations: ["relation" => ["scopeOne", "scopeTwo"]]?

Describe alternatives you've considered

I haven't found any alternative to overcome this issue.

@spawnia spawnia added the enhancement A feature or improvement label Jan 11, 2020
@spawnia
Copy link
Collaborator

spawnia commented Jan 11, 2020

As of very recently,repeatable directives are now allowed in principle. We will have to add support for that: #1133

I think allowing multiple @with will be the best solution for this use case.

@spawnia spawnia added the 5.x Related to the 5.x release series label May 12, 2020
@spawnia
Copy link
Collaborator

spawnia commented Aug 14, 2020

Repeatable use of the directive is not generally the issue here, it should work in principle - and is now also supported by webonyx/graphql-php

As of now, the way the batch loader instantiation works does enforce that there is always a single batch loader per field. Since the relation to load is passed during the initial construction, subsequent uses of @with don't have a chance to register the relation they want to load.

I am working on moving the relation passing out of the constructor, so the relations to load can added to by multiple directives.

@spawnia spawnia removed the 5.x Related to the 5.x release series label Aug 14, 2020
@pfbernardo
Copy link

Hi.

I'm using lighthouse 5.0.

I have the following type

type CorrespondenceLetter {
    id: ID!
    name: String!
    target_objects: [CorrespondenceItemTargetObject!]! @with(relation: "targetContacts") @with(relation: "targetContracts") @field(resolver: "App\\GraphQL\\Types\\CorrespondenceItemType@targetObjects")
}

CorrespondenceItemTargetObject is a Union between 2 types.

On the CorrespondenceItemType@targetObjects field resolver I see that only the last relation "targetContracts" is actually loaded.

@spawnia I think it is because of the problem you describe on your last comment.
Do you have any news on this?

@spawnia spawnia changed the title It should be possible to eager-load multiple relationships through @with Eager-load multiple relationships through @with Jan 9, 2021
@tlaverdure
Copy link
Collaborator

Hey, I'm trying to kill some N+1 issues and was wondering is this still in the works?

I'm specifically trying to eager load multiple relationships and also eager load nested relationships using dot syntax a.b.c, but it doesn't look like its supported yet?

type User {
    recommended: Boolean @with(relation: "a")  @with(relation: "a.b") @with(relation: "a.b.c")
}

@spawnia
Copy link
Collaborator

spawnia commented Jun 5, 2024

I tried reenabling the failing test case for this in #2525 and implemented a fix for it. However, this causes other tests to fail.

The fundamental issue with this is that relations loaded using @with can also have scopes attached, so loading the same relation twice might still be necessary.

@tlaverdure
Copy link
Collaborator

Thanks, I'll take a look when I find some time.

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

Successfully merging a pull request may close this issue.

4 participants